Re: [PATCH v2 03/14] nfsd: move fh_want_write() after preamble in nfsd4_create_file()

On Mon, 2026-07-06 at 08:19 +1000, NeilBrown wrote:
> From: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> 
> As part of separating the nfsd-specific code from the VFS interaction
> code in nfsd4_create_file(), move fh_want_write() to just before we need
> it.
> 
> Consequently errors in the "if" statement that this code is moved over
> can now be returned immediately rather than needing to "goto out".
> 
> Also restructure that "if" statement to only test is_create_with_attrs()
> once.
> 
> Signed-off-by: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> ---
>  fs/nfsd/nfs4proc.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index a93132323b66..17be4f7420fc 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -270,22 +270,18 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	parent = fhp->fh_dentry;
>  	inode = d_inode(parent);
>  
> -	host_err = fh_want_write(fhp);
> -	if (host_err)
> -		return nfserrno(host_err);
> -
> -	if (open->op_acl) {
> +	if (!is_create_with_attrs(open)) {
> +		/* No attrs to check */
> +	} else if (open->op_acl) {
>  		if (open->op_dpacl || open->op_pacl) {
> -			status = nfserr_inval;
> -			goto out;
> +			/* Cannot specify both NFSv4 and Posix ACLs */
> +			return nfserr_inval;
>  		}
> -		if (is_create_with_attrs(open)) {
> -			status = nfsd4_acl_to_attr(NF4REG, open->op_acl,
> +		status = nfsd4_acl_to_attr(NF4REG, open->op_acl,
>  						   &attrs);
> -			if (status)
> -				goto out;
> -		}
> -	} else if (is_create_with_attrs(open)) {
> +		if (status)
> +			return status;
> +	} else {
>  		/* The dpacl and pacl will get released by nfsd_attrs_free(). */
>  		attrs.na_dpacl = open->op_dpacl;
>  		attrs.na_pacl = open->op_pacl;
> @@ -293,6 +289,12 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		open->op_pacl = NULL;
>  	}
>  
> +	host_err = fh_want_write(fhp);
> +	if (host_err) {
> +		status = nfserrno(host_err);
> +		goto out_free;
> +	}
> +
>  	child = start_creating(&nop_mnt_idmap, parent,
>  			       &QSTR_LEN(open->op_fname, open->op_fnamelen));
>  	if (IS_ERR(child)) {
> @@ -409,8 +411,9 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		open->op_bmval[2] &= ~FATTR4_WORD2_POSIX_ACCESS_ACL;
>  out:
>  	end_creating(child);
> -	nfsd_attrs_free(&attrs);
>  	fh_drop_write(fhp);
> +out_free:
> +	nfsd_attrs_free(&attrs);
>  	return status;
>  }
>  

Reviewed-by: Jeff Layton <[email protected]>