* [RFC 1/1] NFSD add vfs_fsync after async copy is done
@ 2021-04-22 16:19 Olga Kornievskaia
2021-04-22 16:54 ` dai.ngo
0 siblings, 1 reply; 5+ messages in thread
From: Olga Kornievskaia @ 2021-04-22 16:19 UTC (permalink / raw)
To: bfields, chuck.lever; +Cc: linux-nfs
From: Olga Kornievskaia <kolga@netapp.com>
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@netapp.com>
---
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(©->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) {
+ down_write(©->nf_dst->nf_rwsem);
+ status = vfs_fsync_range(copy->nf_dst->nf_file,
+ copy->cp_dst_pos, bytes_copied, 0);
+ up_write(©->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;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC 1/1] NFSD add vfs_fsync after async copy is done
2021-04-22 16:19 [RFC 1/1] NFSD add vfs_fsync after async copy is done Olga Kornievskaia
@ 2021-04-22 16:54 ` dai.ngo
2021-04-22 17:27 ` Olga Kornievskaia
0 siblings, 1 reply; 5+ messages in thread
From: dai.ngo @ 2021-04-22 16:54 UTC (permalink / raw)
To: Olga Kornievskaia, bfields, chuck.lever; +Cc: linux-nfs
On 4/22/21 9:19 AM, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
>
> 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@netapp.com>
> ---
> 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(©->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(©->nf_dst->nf_rwsem);
> + status = vfs_fsync_range(copy->nf_dst->nf_file,
> + copy->cp_dst_pos, bytes_copied, 0);
> + up_write(©->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;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC 1/1] NFSD add vfs_fsync after async copy is done
2021-04-22 16:54 ` dai.ngo
@ 2021-04-22 17:27 ` Olga Kornievskaia
2021-04-22 17:37 ` dai.ngo
0 siblings, 1 reply; 5+ messages in thread
From: Olga Kornievskaia @ 2021-04-22 17:27 UTC (permalink / raw)
To: Dai Ngo; +Cc: J. Bruce Fields, Chuck Lever, linux-nfs
On Thu, Apr 22, 2021 at 12:54 PM <dai.ngo@oracle.com> wrote:
>
>
> On 4/22/21 9:19 AM, Olga Kornievskaia wrote:
> > From: Olga Kornievskaia <kolga@netapp.com>
> >
> > 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@netapp.com>
> > ---
> > 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(©->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).
I don't think so. A successful async copy should never be less the
requested bytes.
>
> -Dai
>
> > + down_write(©->nf_dst->nf_rwsem);
> > + status = vfs_fsync_range(copy->nf_dst->nf_file,
> > + copy->cp_dst_pos, bytes_copied, 0);
> > + up_write(©->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;
> > }
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC 1/1] NFSD add vfs_fsync after async copy is done
2021-04-22 17:27 ` Olga Kornievskaia
@ 2021-04-22 17:37 ` dai.ngo
2021-04-22 18:20 ` Olga Kornievskaia
0 siblings, 1 reply; 5+ messages in thread
From: dai.ngo @ 2021-04-22 17:37 UTC (permalink / raw)
To: Olga Kornievskaia; +Cc: J. Bruce Fields, Chuck Lever, linux-nfs
On 4/22/21 10:27 AM, Olga Kornievskaia wrote:
> On Thu, Apr 22, 2021 at 12:54 PM <dai.ngo@oracle.com> wrote:
>>
>> On 4/22/21 9:19 AM, Olga Kornievskaia wrote:
>>> From: Olga Kornievskaia <kolga@netapp.com>
>>>
>>> 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@netapp.com>
>>> ---
>>> 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(©->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).
> I don't think so. A successful async copy should never be less the
> requested bytes.
nfsd_copy_file_range can return 0 on the last write in the loop which
causes the test (bytes_copied > 0) to fail which then skipping the
commit. The check (bytes_total < copy->cp_count) says do the commit
if there are any bytes successfully written so far.
-Dai
>
>> -Dai
>>
>>> + down_write(©->nf_dst->nf_rwsem);
>>> + status = vfs_fsync_range(copy->nf_dst->nf_file,
>>> + copy->cp_dst_pos, bytes_copied, 0);
>>> + up_write(©->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;
>>> }
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC 1/1] NFSD add vfs_fsync after async copy is done
2021-04-22 17:37 ` dai.ngo
@ 2021-04-22 18:20 ` Olga Kornievskaia
0 siblings, 0 replies; 5+ messages in thread
From: Olga Kornievskaia @ 2021-04-22 18:20 UTC (permalink / raw)
To: Dai Ngo; +Cc: J. Bruce Fields, Chuck Lever, linux-nfs
On Thu, Apr 22, 2021 at 1:37 PM <dai.ngo@oracle.com> wrote:
>
>
> On 4/22/21 10:27 AM, Olga Kornievskaia wrote:
> > On Thu, Apr 22, 2021 at 12:54 PM <dai.ngo@oracle.com> wrote:
> >>
> >> On 4/22/21 9:19 AM, Olga Kornievskaia wrote:
> >>> From: Olga Kornievskaia <kolga@netapp.com>
> >>>
> >>> 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@netapp.com>
> >>> ---
> >>> 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(©->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).
> > I don't think so. A successful async copy should never be less the
> > requested bytes.
>
> nfsd_copy_file_range can return 0 on the last write in the loop which
> causes the test (bytes_copied > 0) to fail which then skipping the
> commit. The check (bytes_total < copy->cp_count) says do the commit
> if there are any bytes successfully written so far.
Ops, right, so actually what I used to have "if (bytes_total > 0)". I
think typically we should have bytes_total = copy_cp_count but the
reason I don't have it to be bytes_total <= copy->cp_count is then if
bytes_total=0 it would call the vfs_fsync() which I didn't what to do.
>
> -Dai
>
> >
> >> -Dai
> >>
> >>> + down_write(©->nf_dst->nf_rwsem);
> >>> + status = vfs_fsync_range(copy->nf_dst->nf_file,
> >>> + copy->cp_dst_pos, bytes_copied, 0);
> >>> + up_write(©->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;
> >>> }
> >>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-22 18:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 16:19 [RFC 1/1] NFSD add vfs_fsync after async copy is done Olga Kornievskaia
2021-04-22 16:54 ` dai.ngo
2021-04-22 17:27 ` Olga Kornievskaia
2021-04-22 17:37 ` dai.ngo
2021-04-22 18:20 ` Olga Kornievskaia
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.