Re: [PATCH v2 05/14] nfsd: remove subtlety from nfsd4_create_file()

On Mon, 2026-07-06 at 08:19 +1000, NeilBrown wrote:
> From: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> 
> nfsd4_create_file() has a switch with cases for
> NFS4_CREATE_EXCLUSIVE and NFS4_CREATE_EXCLUSIVE4_1 which are identical
> except for one line which is marked "subtle" in both cases.
> 
> The difference boils down to a "goto".  For the EXCLUSIVE case the
> target is "out:" which is after a setattr call.  For EXCLUSIVE4_1
> the target is "set_attr:" which is the start of that setattr call.
> 
> In the EXCLUSIVE case 'attrs' will only contain the verifier.  Setting
> these again is not harmful as discussed in the previous patch.  It will
> also call commit_metadata().  In performance terms the cost of an extra
> 'commit' in the rare case of a replaying exclusive create is negligible.
> 
> So we can safely "goto setattr" in both cases and thus simplify the
> code.
> 
> Signed-off-by: NeilBrown <neil-+NVA1uvv1dVBDLzU/[email protected]>
> ---
>  fs/nfsd/nfs4proc.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index b723ba08ddaf..69cdbdcde7e9 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -363,22 +363,15 @@ nfsd4_create_file(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  			status = nfserr_exist;
>  			break;
>  		case NFS4_CREATE_EXCLUSIVE:
> -			if (inode_get_mtime_sec(d_inode(child)) == v_mtime &&
> -			    inode_get_atime_sec(d_inode(child)) == v_atime &&
> -			    d_inode(child)->i_size == 0) {
> -				open->op_created = true;
> -				break;		/* subtle */
> -			}
> -			status = nfserr_exist;
> -			break;
>  		case NFS4_CREATE_EXCLUSIVE4_1:
>  			if (inode_get_mtime_sec(d_inode(child)) == v_mtime &&
>  			    inode_get_atime_sec(d_inode(child)) == v_atime &&
>  			    d_inode(child)->i_size == 0) {
>  				open->op_created = true;
> -				goto set_attr;	/* subtle */
> +				goto set_attr;
>  			}
>  			status = nfserr_exist;
> +			break;
>  		}
>  		goto out;
>  	}

Nice cleanup:

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