Re: [PATCH v2 04/14] nfsd: move more nfs-specific code into preamble of nfsd4_create_file()

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2026-07-06 at 08:19 +1000, NeilBrown wrote:
> From: NeilBrown <neil@brown.name>
> 
> Do NFS-specific prep before interacting with the VFS.
> 
> We now add the verifier to iap early so it applies even when an
> EXCLUSIVE4_1 replay is detected based on that verifier, so we will set
> those attributes again.  This should be harmless even though it will
> update ctime and i_version, and so will update the changeid seen by the
> client.  It shouldn't matter because the resend implies that the client
> hasn't seen the file or its changeid.  If some other client happens to
> have noticed the file, it might see an unnecessary changeid up, but that
> is of no consequence.
> 
> Note that ctime would have been updated anyway if the client has
> included other attributes like an ACL.
> 
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>  fs/nfsd/nfs4proc.c | 55 +++++++++++++++++++++++-----------------------
>  1 file changed, 27 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 17be4f7420fc..b723ba08ddaf 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -270,6 +270,9 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	parent = fhp->fh_dentry;
>  	inode = d_inode(parent);
>  
> +	if (!IS_POSIXACL(inode))
> +		iap->ia_mode &= ~current_umask();
> +
>  	if (!is_create_with_attrs(open)) {
>  		/* No attrs to check */
>  	} else if (open->op_acl) {
> @@ -289,6 +292,30 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		open->op_pacl = NULL;
>  	}
>  
> +	v_mtime = 0;
> +	v_atime = 0;
> +	if (nfsd4_create_is_exclusive(open->op_createmode)) {
> +		u32 *verifier = (u32 *)open->op_verf.data;
> +
> +		/*
> +		 * Solaris 7 gets confused (bugid 4218508) if these have
> +		 * the high bit set, as do xfs filesystems without the
> +		 * "bigtime" feature. So just clear the high bits. If this
> +		 * is ever changed to use different attrs for storing the
> +		 * verifier, then do_open_lookup() will also need to be
> +		 * fixed accordingly.
> +		 */
> +		v_mtime = verifier[0] & 0x7fffffff;
> +		v_atime = verifier[1] & 0x7fffffff;
> +
> +		iap->ia_valid |= ATTR_MTIME | ATTR_ATIME |
> +				 ATTR_MTIME_SET|ATTR_ATIME_SET;
> +		iap->ia_mtime.tv_sec = v_mtime;
> +		iap->ia_atime.tv_sec = v_atime;
> +		iap->ia_mtime.tv_nsec = 0;
> +		iap->ia_atime.tv_nsec = 0;
> +	}
> +
>  	host_err = fh_want_write(fhp);
>  	if (host_err) {
>  		status = nfserrno(host_err);
> @@ -308,23 +335,6 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  			goto out;
>  	}
>  
> -	v_mtime = 0;
> -	v_atime = 0;
> -	if (nfsd4_create_is_exclusive(open->op_createmode)) {
> -		u32 *verifier = (u32 *)open->op_verf.data;
> -
> -		/*
> -		 * Solaris 7 gets confused (bugid 4218508) if these have
> -		 * the high bit set, as do xfs filesystems without the
> -		 * "bigtime" feature. So just clear the high bits. If this
> -		 * is ever changed to use different attrs for storing the
> -		 * verifier, then do_open_lookup() will also need to be
> -		 * fixed accordingly.
> -		 */
> -		v_mtime = verifier[0] & 0x7fffffff;
> -		v_atime = verifier[1] & 0x7fffffff;
> -	}
> -
>  	if (d_really_is_positive(child)) {
>  		/* NFSv4 protocol requires change attributes even though
>  		 * no change happened.
> @@ -373,9 +383,6 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		goto out;
>  	}
>  
> -	if (!IS_POSIXACL(inode))
> -		iap->ia_mode &= ~current_umask();
> -
>  	status = nfsd4_vfs_create(fhp, &child, open);
>  	if (status != nfs_ok)
>  		goto out;
> @@ -389,14 +396,6 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  	/* A newly created file already has a file size of zero. */
>  	if ((iap->ia_valid & ATTR_SIZE) && (iap->ia_size == 0))
>  		iap->ia_valid &= ~ATTR_SIZE;
> -	if (nfsd4_create_is_exclusive(open->op_createmode)) {
> -		iap->ia_valid |= ATTR_MTIME | ATTR_ATIME |
> -				 ATTR_MTIME_SET|ATTR_ATIME_SET;
> -		iap->ia_mtime.tv_sec = v_mtime;
> -		iap->ia_atime.tv_sec = v_atime;
> -		iap->ia_mtime.tv_nsec = 0;
> -		iap->ia_atime.tv_nsec = 0;
> -	}
>  
>  set_attr:
>  	status = nfsd_create_setattr(rqstp, fhp, resfhp, &attrs);

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