2021-05-19 20:20:43

by Olga Kornievskaia

Subject: [PATCH v2 1/1] NFSD add vfs_fsync after async copy is done

From: Olga Kornievskaia <[email protected]>

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.

--- v2: moved the committed argument into the nfsd4_copy structure

Signed-off-by: Olga Kornievskaia <[email protected]>
---
fs/nfsd/nfs4proc.c | 14 +++++++++++++-
fs/nfsd/xdr4.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index f4ce93d7f26e..dc7ac4b39eff 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1375,7 +1375,8 @@ static const struct nfsd4_callback_ops nfsd4_cb_offload_ops = {

static void nfsd4_init_copy_res(struct nfsd4_copy *copy, bool sync)
{
- copy->cp_res.wr_stable_how = NFS_UNSTABLE;
+ copy->cp_res.wr_stable_how =
+ copy->committed ? NFS_FILE_SYNC : NFS_UNSTABLE;
copy->cp_synchronous = sync;
gen_boot_verifier(&copy->cp_res.wr_verifier, copy->cp_clp->net);
}
@@ -1386,6 +1387,7 @@ static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy)
u64 bytes_total = copy->cp_count;
u64 src_pos = copy->cp_src_pos;
u64 dst_pos = copy->cp_dst_pos;
+ __be32 status;

/* See RFC 7862 p.67: */
if (bytes_total == 0)
@@ -1403,6 +1405,16 @@ 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 && copy->cp_res.wr_bytes_written > 0) {
+ down_write(&copy->nf_dst->nf_rwsem);
+ status = vfs_fsync_range(copy->nf_dst->nf_file,
+ copy->cp_dst_pos,
+ copy->cp_res.wr_bytes_written, 0);
+ up_write(&copy->nf_dst->nf_rwsem);
+ if (!status)
+ copy->committed = true;
+ }
return bytes_copied;
}

diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index a7c425254fee..3e4052e3bd50 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -567,6 +567,7 @@ struct nfsd4_copy {
struct vfsmount *ss_mnt;
struct nfs_fh c_fh;
nfs4_stateid stateid;
+ bool committed;
};

struct nfsd4_seek {
--
2.27.0

2021-05-19 21:35:45

by J. Bruce Fields

Subject: Re: [PATCH v2 1/1] NFSD add vfs_fsync after async copy is done

On Wed, May 19, 2021 at 02:48:27PM -0400, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <[email protected]>
>
> 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.
>
> --- v2: moved the committed argument into the nfsd4_copy structure

Makes sense to me, thanks. Applying if nobody sees an objection.--b.

>
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> fs/nfsd/nfs4proc.c | 14 +++++++++++++-
> fs/nfsd/xdr4.h | 1 +
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index f4ce93d7f26e..dc7ac4b39eff 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1375,7 +1375,8 @@ static const struct nfsd4_callback_ops nfsd4_cb_offload_ops = {
>
> static void nfsd4_init_copy_res(struct nfsd4_copy *copy, bool sync)
> {
> - copy->cp_res.wr_stable_how = NFS_UNSTABLE;
> + copy->cp_res.wr_stable_how =
> + copy->committed ? NFS_FILE_SYNC : NFS_UNSTABLE;
> copy->cp_synchronous = sync;
> gen_boot_verifier(&copy->cp_res.wr_verifier, copy->cp_clp->net);
> }
> @@ -1386,6 +1387,7 @@ static ssize_t _nfsd_copy_file_range(struct nfsd4_copy *copy)
> u64 bytes_total = copy->cp_count;
> u64 src_pos = copy->cp_src_pos;
> u64 dst_pos = copy->cp_dst_pos;
> + __be32 status;
>
> /* See RFC 7862 p.67: */
> if (bytes_total == 0)
> @@ -1403,6 +1405,16 @@ 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 && copy->cp_res.wr_bytes_written > 0) {
> + down_write(&copy->nf_dst->nf_rwsem);
> + status = vfs_fsync_range(copy->nf_dst->nf_file,
> + copy->cp_dst_pos,
> + copy->cp_res.wr_bytes_written, 0);
> + up_write(&copy->nf_dst->nf_rwsem);
> + if (!status)
> + copy->committed = true;
> + }
> return bytes_copied;
> }
>
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index a7c425254fee..3e4052e3bd50 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -567,6 +567,7 @@ struct nfsd4_copy {
> struct vfsmount *ss_mnt;
> struct nfs_fh c_fh;
> nfs4_stateid stateid;
> + bool committed;
> };
>
> struct nfsd4_seek {
> --
> 2.27.0