Re: [RFC PATCH 1/7] NFSD: Async COPY result needs to return a write verifier

[Jeff Layton <jlayton@xxxxxxxxxx>]

On Wed, 2024-08-28 at 13:40 -0400, cel@xxxxxxxxxx wrote:
> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
> 
> Currently, when NFSD handles an asynchronous COPY, it returns a
> zero write verifier, relying on the subsequent CB_OFFLOAD callback
> to pass the write verifier and a stable_how4 value to the client.
> 
> However, if the CB_OFFLOAD never arrives at the client (for example,
> if a network partition occurs just as the server sends the
> CB_OFFLOAD operation), the client will never receive this verifier.
> Thus, if the client sends a follow-up COMMIT, there is no way for
> the client to assess the COMMIT result.
> 
> The usual recovery for a missing CB_OFFLOAD is for the client to
> send an OFFLOAD_STATUS operation, but that operation does not carry
> a write verifier in its result. Neither does it carry a stable_how4
> value, so the client /must/ send a COMMIT in this case -- which will
> always fail because currently there's still no write verifier in the
> COPY result.
> 
> Thus the server needs to return a normal write verifier in its COPY
> result even if the COPY operation is to be performed asynchronously.
> 
> If the server recognizes the callback stateid in subsequent
> OFFLOAD_STATUS operations, then obviously it has not restarted, and
> the write verifier the client received in the COPY result is still
> valid and can be used to assess a COMMIT of the copied data, if one
> is needed.
> 
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
>  fs/nfsd/nfs4proc.c | 25 +++++++++----------------
>  1 file changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 2e39cf2e502a..60c526adc27c 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -751,15 +751,6 @@ nfsd4_access(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  			   &access->ac_supported);
>  }
>  
> -static void gen_boot_verifier(nfs4_verifier *verifier, struct net *net)
> -{
> -	__be32 *verf = (__be32 *)verifier->data;
> -
> -	BUILD_BUG_ON(2*sizeof(*verf) != sizeof(verifier->data));
> -
> -	nfsd_copy_write_verifier(verf, net_generic(net, nfsd_net_id));
> -}
> -
>  static __be32
>  nfsd4_commit(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	     union nfsd4_op_u *u)
> @@ -1630,7 +1621,6 @@ static void nfsd4_init_copy_res(struct nfsd4_copy *copy, bool sync)
>  		test_bit(NFSD4_COPY_F_COMMITTED, &copy->cp_flags) ?
>  			NFS_FILE_SYNC : NFS_UNSTABLE;
>  	nfsd4_copy_set_sync(copy, sync);
> -	gen_boot_verifier(&copy->cp_res.wr_verifier, copy->cp_clp->net);
>  }
>  
>  static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy,
> @@ -1803,9 +1793,11 @@ static __be32
>  nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		union nfsd4_op_u *u)
>  {
> -	struct nfsd4_copy *copy = &u->copy;
> -	__be32 status;
> +	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>  	struct nfsd4_copy *async_copy = NULL;
> +	struct nfsd4_copy *copy = &u->copy;
> +	struct nfsd42_write_res *result;
> +	__be32 status;
>  
>  	/*
>  	 * Currently, async COPY is not reliable. Force all COPY
> @@ -1814,6 +1806,9 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	 */
>  	nfsd4_copy_set_sync(copy, true);
>  
> +	result = &copy->cp_res;
> +	nfsd_copy_write_verifier((__be32 *)&result->wr_verifier.data, nn);
> +
>  	copy->cp_clp = cstate->clp;
>  	if (nfsd4_ssc_is_inter(copy)) {
>  		trace_nfsd_copy_inter(copy);
> @@ -1838,8 +1833,6 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	memcpy(&copy->fh, &cstate->current_fh.fh_handle,
>  		sizeof(struct knfsd_fh));
>  	if (nfsd4_copy_is_async(copy)) {
> -		struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> -
>  		status = nfserrno(-ENOMEM);
>  		async_copy = kzalloc(sizeof(struct nfsd4_copy), GFP_KERNEL);
>  		if (!async_copy)
> @@ -1851,8 +1844,8 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  			goto out_err;
>  		if (!nfs4_init_copy_state(nn, copy))
>  			goto out_err;
> -		memcpy(&copy->cp_res.cb_stateid, &copy->cp_stateid.cs_stid,
> -			sizeof(copy->cp_res.cb_stateid));
> +		memcpy(&result->cb_stateid, &copy->cp_stateid.cs_stid,
> +			sizeof(result->cb_stateid));
>  		dup_copy_fields(copy, async_copy);
>  		async_copy->copy_task = kthread_create(nfsd4_do_async_copy,
>  				async_copy, "%s", "copy thread");

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>