Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Apr 2023 10:16:27 -0400
From:   Chuck Lever <cel@kernel.org>
To:     Amir Goldstein <amir73il@gmail.com>
Cc:     Jan Kara <jack@suse.cz>, Christian Brauner <brauner@kernel.org>,
        Miklos Szeredi <miklos@szeredi.hu>,
        linux-unionfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-api@vger.kernel.org, linux-nfs@vger.kernel.org,
        jlayton@kernel.org
Subject: Re: [RFC][PATCH 1/4] exportfs: change connectable argument to bit
 flags
Message-ID: <ZEkyO1xnmhBaGLgE@manet.1015granger.net>
References: <20230425130105.2606684-1-amir73il@gmail.com>
 <20230425130105.2606684-2-amir73il@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230425130105.2606684-2-amir73il@gmail.com>
Precedence: bulk

On Tue, Apr 25, 2023 at 04:01:02PM +0300, Amir Goldstein wrote:
> Convert the bool connectable arguemnt into a bit flags argument and
> define the EXPORT_FS_CONNECTABLE flag as a requested property of the
> file handle.
> 
> We are going to add a flag for requesting non-decodeable file handles.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/exportfs/expfs.c      | 11 +++++++++--
>  fs/nfsd/nfsfh.c          |  5 +++--
>  include/linux/exportfs.h |  6 ++++--
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> index ab88d33d106c..bf1b4925fedd 100644
> --- a/fs/exportfs/expfs.c
> +++ b/fs/exportfs/expfs.c
> @@ -393,14 +393,21 @@ int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  }
>  EXPORT_SYMBOL_GPL(exportfs_encode_inode_fh);
>  
> +/**
> + * exportfs_encode_fh - encode a file handle from dentry
> + * @dentry:  the object to encode
> + * @fid:     where to store the file handle fragment
> + * @max_len: maximum length to store there
> + * @flags:   properties of the requrested file handle

Thanks for adding the KDOC comment!

/requrested/requested

And please add:

 *
 * Returns an enum fid_type or a negative errno

> + */
>  int exportfs_encode_fh(struct dentry *dentry, struct fid *fid, int *max_len,
> -		int connectable)
> +		       int flags)
>  {
>  	int error;
>  	struct dentry *p = NULL;
>  	struct inode *inode = dentry->d_inode, *parent = NULL;
>  
> -	if (connectable && !S_ISDIR(inode->i_mode)) {
> +	if ((flags & EXPORT_FH_CONNECTABLE) && !S_ISDIR(inode->i_mode)) {
>  		p = dget_parent(dentry);
>  		/*
>  		 * note that while p might've ceased to be our parent already,
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index ccd8485fee04..31e4505c0df3 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -414,10 +414,11 @@ static void _fh_update(struct svc_fh *fhp, struct svc_export *exp,
>  		struct fid *fid = (struct fid *)
>  			(fhp->fh_handle.fh_fsid + fhp->fh_handle.fh_size/4 - 1);
>  		int maxsize = (fhp->fh_maxsize - fhp->fh_handle.fh_size)/4;
> -		int subtreecheck = !(exp->ex_flags & NFSEXP_NOSUBTREECHECK);
> +		int fh_flags = (exp->ex_flags & NFSEXP_NOSUBTREECHECK) ? 0 :
> +				EXPORT_FH_CONNECTABLE;
>  
>  		fhp->fh_handle.fh_fileid_type =
> -			exportfs_encode_fh(dentry, fid, &maxsize, subtreecheck);
> +			exportfs_encode_fh(dentry, fid, &maxsize, fh_flags);
>  		fhp->fh_handle.fh_size += maxsize * 4;
>  	} else {
>  		fhp->fh_handle.fh_fileid_type = FILEID_ROOT;
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 601700fedc91..2b1048238170 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -135,6 +135,8 @@ struct fid {
>  	};
>  };
>  
> +#define EXPORT_FH_CONNECTABLE	0x1
> +
>  /**
>   * struct export_operations - for nfsd to communicate with file systems
>   * @encode_fh:      encode a file handle fragment from a dentry
> @@ -150,7 +152,7 @@ struct fid {
>   * encode_fh:
>   *    @encode_fh should store in the file handle fragment @fh (using at most
>   *    @max_len bytes) information that can be used by @decode_fh to recover the
> - *    file referred to by the &struct dentry @de.  If the @connectable flag is
> + *    file referred to by the &struct dentry @de.  If @flag has CONNECTABLE bit
>   *    set, the encode_fh() should store sufficient information so that a good
>   *    attempt can be made to find not only the file but also it's place in the
>   *    filesystem.   This typically means storing a reference to de->d_parent in
> @@ -226,7 +228,7 @@ struct export_operations {
>  extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  				    int *max_len, struct inode *parent);
>  extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
> -	int *max_len, int connectable);
> +			      int *max_len, int flags);
>  extern struct dentry *exportfs_decode_fh_raw(struct vfsmount *mnt,
>  					     struct fid *fid, int fh_len,
>  					     int fileid_type,
> -- 
> 2.34.1
>