Re: [RFC 1/1] NFSD add vfs_fsync after async copy is done

[dai.ngo@xxxxxxxxxx]

On 4/22/21 9:19 AM, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@xxxxxxxxxx>
>
> Currently, the server does all copies as NFS_UNSTABLE. For synchronous
> copies linux client will append a COMMIT to the COPY compound but for
> async copies it does not (because COMMIT needs to be done after all
> bytes are copied and not as a reply to the COPY operation).
>
> However, in order to save the client doing a COMMIT as a separate
> rpc, the server can reply back with NFS_FILE_SYNC copy. This patch
> proposed to add vfs_fsync() call at the end of the async copy.
>
> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
> ---
>   fs/nfsd/nfs4proc.c | 22 +++++++++++++++++-----
>   1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 66dea2f1eed8..c6f45f67d59b 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1536,19 +1536,21 @@ static const struct nfsd4_callback_ops nfsd4_cb_offload_ops = {
>   	.done = nfsd4_cb_offload_done
>   };
>   
> -static void nfsd4_init_copy_res(struct nfsd4_copy *copy, bool sync)
> +static void nfsd4_init_copy_res(struct nfsd4_copy *copy, bool sync,
> +				bool committed)
>   {
> -	copy->cp_res.wr_stable_how = NFS_UNSTABLE;
> +	copy->cp_res.wr_stable_how = committed ? NFS_FILE_SYNC : NFS_UNSTABLE;
>   	copy->cp_synchronous = sync;
>   	gen_boot_verifier(&copy->cp_res.wr_verifier, copy->cp_clp->net);
>   }
>   
> -static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy)
> +static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy, bool *committed)
>   {
>   	ssize_t bytes_copied = 0;
>   	size_t bytes_total = copy->cp_count;
>   	u64 src_pos = copy->cp_src_pos;
>   	u64 dst_pos = copy->cp_dst_pos;
> +	__be32 status;
>   
>   	do {
>   		if (kthread_should_stop())
> @@ -1563,6 +1565,15 @@ static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy)
>   		src_pos += bytes_copied;
>   		dst_pos += bytes_copied;
>   	} while (bytes_total > 0 && !copy->cp_synchronous);
> +	/* for a non-zero asynchronous copy do a commit of data */
> +	if (!copy->cp_synchronous && bytes_copied > 0) {

I think (bytes_copied > 0) should be (bytes_total < copy->cp_count).

-Dai

> +		down_write(&copy->nf_dst->nf_rwsem);
> +		status = vfs_fsync_range(copy->nf_dst->nf_file,
> +					 copy->cp_dst_pos, bytes_copied, 0);
> +		up_write(&copy->nf_dst->nf_rwsem);
> +		if (!status)
> +			*committed = true;
> +	}
>   	return bytes_copied;
>   }
>   
> @@ -1570,15 +1581,16 @@ static __be32 nfsd4_do_copy(struct nfsd4_copy *copy, bool sync)
>   {
>   	__be32 status;
>   	ssize_t bytes;
> +	bool committed = false;
>   
> -	bytes = _nfsd_copy_file_range(copy);
> +	bytes = _nfsd_copy_file_range(copy, &committed);
>   	/* for async copy, we ignore the error, client can always retry
>   	 * to get the error
>   	 */
>   	if (bytes < 0 && !copy->cp_res.wr_bytes_written)
>   		status = nfserrno(bytes);
>   	else {
> -		nfsd4_init_copy_res(copy, sync);
> +		nfsd4_init_copy_res(copy, sync, committed);
>   		status = nfs_ok;
>   	}
>