2022-09-08 16:46:22

by Jeff Layton

[permalink] [raw]
Subject: [PATCH] nfsd: clean up mounted_on_fileid handling

We only need the inode number for this, not a full rack of attributes.
Rename this function make it take a pointer to a u64 instead of
struct kstat, and change it to just request STATX_INO.

Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfsd/nfs4xdr.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 1e9690a061ec..5980df859c3a 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2774,9 +2774,10 @@ static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *bmval2, u32
}


-static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
+static int get_mounted_on_ino(struct svc_export *exp, u64 *pino)
{
struct path path = exp->ex_path;
+ struct kstat stat;
int err;

path_get(&path);
@@ -2784,8 +2785,10 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
if (path.dentry != path.mnt->mnt_root)
break;
}
- err = vfs_getattr(&path, stat, STATX_BASIC_STATS, AT_STATX_SYNC_AS_STAT);
+ err = vfs_getattr(&path, &stat, STATX_INO, AT_STATX_SYNC_AS_STAT);
path_put(&path);
+ if (!err)
+ *pino = stat.ino;
return err;
}

@@ -3282,22 +3285,21 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
*p++ = cpu_to_be32(stat.btime.tv_nsec);
}
if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
- struct kstat parent_stat;
u64 ino = stat.ino;

p = xdr_reserve_space(xdr, 8);
if (!p)
goto out_resource;
/*
- * Get parent's attributes if not ignoring crossmount
- * and this is the root of a cross-mounted filesystem.
+ * Get ino of mountpoint in parent filesystem, if not ignoring
+ * crossmount and this is the root of a cross-mounted
+ * filesystem.
*/
if (ignore_crossmnt == 0 &&
dentry == exp->ex_path.mnt->mnt_root) {
- err = get_parent_attributes(exp, &parent_stat);
+ err = get_mounted_on_ino(exp, &ino);
if (err)
goto out_nfserr;
- ino = parent_stat.ino;
}
p = xdr_encode_hyper(p, ino);
}
--
2.37.3


2022-09-08 17:15:54

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH] nfsd: clean up mounted_on_fileid handling

On Thu, 2022-09-08 at 12:31 -0400, Jeff Layton wrote:
> We only need the inode number for this, not a full rack of attributes.
> Rename this function make it take a pointer to a u64 instead of
> struct kstat, and change it to just request STATX_INO.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/nfsd/nfs4xdr.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 1e9690a061ec..5980df859c3a 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2774,9 +2774,10 @@ static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *bmval2, u32
> }
>
>
> -static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
> +static int get_mounted_on_ino(struct svc_export *exp, u64 *pino)
> {
> struct path path = exp->ex_path;
> + struct kstat stat;
> int err;
>
> path_get(&path);
> @@ -2784,8 +2785,10 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
> if (path.dentry != path.mnt->mnt_root)
> break;
> }
> - err = vfs_getattr(&path, stat, STATX_BASIC_STATS, AT_STATX_SYNC_AS_STAT);
> + err = vfs_getattr(&path, &stat, STATX_INO, AT_STATX_SYNC_AS_STAT);

We could also consider using AT_STATX_DONT_SYNC here as well, but it's
probably not worthwhile. Just asking for the inode number shouldn't
result in any I/O in a sanely-written getattr.

> path_put(&path);
> + if (!err)
> + *pino = stat.ino;
> return err;
> }
>
> @@ -3282,22 +3285,21 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> *p++ = cpu_to_be32(stat.btime.tv_nsec);
> }
> if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
> - struct kstat parent_stat;
> u64 ino = stat.ino;
>
> p = xdr_reserve_space(xdr, 8);
> if (!p)
> goto out_resource;
> /*
> - * Get parent's attributes if not ignoring crossmount
> - * and this is the root of a cross-mounted filesystem.
> + * Get ino of mountpoint in parent filesystem, if not ignoring
> + * crossmount and this is the root of a cross-mounted
> + * filesystem.
> */
> if (ignore_crossmnt == 0 &&
> dentry == exp->ex_path.mnt->mnt_root) {
> - err = get_parent_attributes(exp, &parent_stat);
> + err = get_mounted_on_ino(exp, &ino);
> if (err)
> goto out_nfserr;
> - ino = parent_stat.ino;
> }
> p = xdr_encode_hyper(p, ino);
> }

--
Jeff Layton <[email protected]>

2022-09-08 19:26:32

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] nfsd: clean up mounted_on_fileid handling



> On Sep 8, 2022, at 12:31 PM, Jeff Layton <[email protected]> wrote:
>
> We only need the inode number for this, not a full rack of attributes.
> Rename this function make it take a pointer to a u64 instead of
> struct kstat, and change it to just request STATX_INO.
>
> Signed-off-by: Jeff Layton <[email protected]>

I'm relying on your expertise in this area.

As I apply this for testing, may I rename get_mounted_on_ino() to
nfsd4_get_mounted_on_ino() ?


> ---
> fs/nfsd/nfs4xdr.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 1e9690a061ec..5980df859c3a 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2774,9 +2774,10 @@ static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *bmval2, u32
> }
>
>
> -static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
> +static int get_mounted_on_ino(struct svc_export *exp, u64 *pino)
> {
> struct path path = exp->ex_path;
> + struct kstat stat;
> int err;
>
> path_get(&path);
> @@ -2784,8 +2785,10 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
> if (path.dentry != path.mnt->mnt_root)
> break;
> }
> - err = vfs_getattr(&path, stat, STATX_BASIC_STATS, AT_STATX_SYNC_AS_STAT);
> + err = vfs_getattr(&path, &stat, STATX_INO, AT_STATX_SYNC_AS_STAT);
> path_put(&path);
> + if (!err)
> + *pino = stat.ino;
> return err;
> }
>
> @@ -3282,22 +3285,21 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> *p++ = cpu_to_be32(stat.btime.tv_nsec);
> }
> if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
> - struct kstat parent_stat;
> u64 ino = stat.ino;
>
> p = xdr_reserve_space(xdr, 8);
> if (!p)
> goto out_resource;
> /*
> - * Get parent's attributes if not ignoring crossmount
> - * and this is the root of a cross-mounted filesystem.
> + * Get ino of mountpoint in parent filesystem, if not ignoring
> + * crossmount and this is the root of a cross-mounted
> + * filesystem.
> */
> if (ignore_crossmnt == 0 &&
> dentry == exp->ex_path.mnt->mnt_root) {
> - err = get_parent_attributes(exp, &parent_stat);
> + err = get_mounted_on_ino(exp, &ino);
> if (err)
> goto out_nfserr;
> - ino = parent_stat.ino;
> }
> p = xdr_encode_hyper(p, ino);
> }
> --
> 2.37.3
>

--
Chuck Lever