2023-04-26 14:19:05

by Chuck Lever

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/4] exportfs: change connectable argument to bit flags

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 <[email protected]>
> ---
> 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
>