Re: [PATCH 1/8] NFSD: drop rqstp arg to do_set_nfs4_acl()

From: Jeff Layton <jlayton@kernel.org>
To: NeilBrown <neilb@suse.de>, Chuck Lever <chuck.lever@oracle.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/8] NFSD: drop rqstp arg to do_set_nfs4_acl()
Date: Wed, 06 Jul 2022 09:17:08 -0400	[thread overview]
Message-ID: <d4ffc3c6e513cb3b1a500b5c2c23700cbf3f4de4.camel@kernel.org> (raw)
In-Reply-To: <165708109255.1940.15894552631928142042.stgit@noble.brown>

On Wed, 2022-07-06 at 14:18 +1000, NeilBrown wrote:
> do_set_nfs4_acl() only needs rqstp to pass to nfsd4_set_nfs4_acl()
> 
> The latter only needs the rqstp to pass to fh_verify().
> 
> In every case that do_set_nfs4_acl() is called, fh_verify() is not
> needed.  It is only needed for filehandles received from the client, the
> filehandles passed to do_set_nfs4_acl() have just been constructed by
> the server, and so must be valid.
> 
> So we can change nfsd4_set_nfs4_acl() to only call fh_verify() is rqstp
> is not NULL, and always pass NULL from do_set_nfs4_acl().
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/nfsd/nfs4acl.c  |   12 +++++++-----
>  fs/nfsd/nfs4proc.c |    9 ++++-----
>  2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
> index eaa3a0cf38f1..5c9b7e01e8ca 100644
> --- a/fs/nfsd/nfs4acl.c
> +++ b/fs/nfsd/nfs4acl.c
> @@ -753,7 +753,7 @@ static int nfs4_acl_nfsv4_to_posix(struct nfs4_acl *acl,
>  
>  __be32
>  nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
> -		struct nfs4_acl *acl)
> +		   struct nfs4_acl *acl)
>  {
>  	__be32 error;
>  	int host_error;
> @@ -762,10 +762,12 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	struct posix_acl *pacl = NULL, *dpacl = NULL;
>  	unsigned int flags = 0;
>  
> -	/* Get inode */
> -	error = fh_verify(rqstp, fhp, 0, NFSD_MAY_SATTR);
> -	if (error)
> -		return error;
> +	if (rqstp) {
> +		/* Get inode */
> +		error = fh_verify(rqstp, fhp, 0, NFSD_MAY_SATTR);
> +		if (error)
> +			return error;
> +	}
>  
>  	dentry = fhp->fh_dentry;
>  	inode = d_inode(dentry);
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 5af9f8d1feb6..60591ceb4985 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -163,12 +163,11 @@ is_create_with_attrs(struct nfsd4_open *open)
>   * in the returned attr bitmap.
>   */
>  static void
> -do_set_nfs4_acl(struct svc_rqst *rqstp, struct svc_fh *fhp,
> -		struct nfs4_acl *acl, u32 *bmval)
> +do_set_nfs4_acl(struct svc_fh *fhp, struct nfs4_acl *acl, u32 *bmval)
>  {
>  	__be32 status;
>  
> -	status = nfsd4_set_nfs4_acl(rqstp, fhp, acl);
> +	status = nfsd4_set_nfs4_acl(NULL, fhp, acl);
>  	if (status)
>  		/*
>  		 * We should probably fail the whole open at this point,
> @@ -474,7 +473,7 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru
>  		goto out;
>  
>  	if (is_create_with_attrs(open) && open->op_acl != NULL)
> -		do_set_nfs4_acl(rqstp, *resfh, open->op_acl, open->op_bmval);
> +		do_set_nfs4_acl(*resfh, open->op_acl, open->op_bmval);
>  
>  	nfsd4_set_open_owner_reply_cache(cstate, open, *resfh);
>  	accmode = NFSD_MAY_NOP;
> @@ -861,7 +860,7 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		nfsd4_security_inode_setsecctx(&resfh, &create->cr_label, create->cr_bmval);
>  
>  	if (create->cr_acl != NULL)
> -		do_set_nfs4_acl(rqstp, &resfh, create->cr_acl,
> +		do_set_nfs4_acl(&resfh, create->cr_acl,
>  				create->cr_bmval);
>  
>  	fh_unlock(&cstate->current_fh);
> 
> 

Reviewed-by: Jeff Layton <jlayton@kernel.org>