Re: [PATCH] nfsd: clean up mounted_on_fileid handling

[Jeff Layton <jlayton@kernel.org>]

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 <jlayton@kernel.org>
> ---
>  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 <jlayton@kernel.org>