2020-11-30 21:29:02

by Dai Ngo

[permalink] [raw]
Subject: [PATCH] NFSD: Fix 5 seconds delay when doing inter server copy

Since commit b4868b44c5628 ("NFSv4: Wait for stateid updates after
CLOSE/OPEN_DOWNGRADE"), every inter server copy operation suffers 5
seconds delay regardless of the size of the copy. The delay is from
nfs_set_open_stateid_locked when the check by nfs_stateid_is_sequential
fails because the seqid in both nfs4_state and nfs4_stateid are 0.

Fix by modifying nfs4_init_cp_state to return the stateid with seqid 1
instead of 0. This is also to conform with section 4.8 of RFC 7862.

Here is the relevant paragraph from section 4.8 of RFC 7862:

A copy offload stateid's seqid MUST NOT be zero. In the context of a
copy offload operation, it is inappropriate to indicate "the most
recent copy offload operation" using a stateid with a seqid of zero
(see Section 8.2.2 of [RFC5661]). It is inappropriate because the
stateid refers to internal state in the server and there may be
several asynchronous COPY operations being performed in parallel on
the same file by the server. Therefore, a copy offload stateid with
a seqid of zero MUST be considered invalid.

Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
Signed-off-by: Dai Ngo <[email protected]>
---
fs/nfsd/nfs4state.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d7f27ed6b794..47006eec724e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -769,6 +769,7 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
spin_lock(&nn->s2s_cp_lock);
new_id = idr_alloc_cyclic(&nn->s2s_cp_stateids, stid, 0, 0, GFP_NOWAIT);
stid->stid.si_opaque.so_id = new_id;
+ stid->stid.si_generation = 1;
spin_unlock(&nn->s2s_cp_lock);
idr_preload_end();
if (new_id < 0)
--
2.9.5


2020-11-30 23:02:46

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] NFSD: Fix 5 seconds delay when doing inter server copy



> On Nov 30, 2020, at 4:24 PM, Dai Ngo <[email protected]> wrote:
>
> Since commit b4868b44c5628 ("NFSv4: Wait for stateid updates after
> CLOSE/OPEN_DOWNGRADE"), every inter server copy operation suffers 5
> seconds delay regardless of the size of the copy. The delay is from
> nfs_set_open_stateid_locked when the check by nfs_stateid_is_sequential
> fails because the seqid in both nfs4_state and nfs4_stateid are 0.
>
> Fix by modifying nfs4_init_cp_state to return the stateid with seqid 1
> instead of 0. This is also to conform with section 4.8 of RFC 7862.
>
> Here is the relevant paragraph from section 4.8 of RFC 7862:
>
> A copy offload stateid's seqid MUST NOT be zero. In the context of a
> copy offload operation, it is inappropriate to indicate "the most
> recent copy offload operation" using a stateid with a seqid of zero
> (see Section 8.2.2 of [RFC5661]). It is inappropriate because the
> stateid refers to internal state in the server and there may be
> several asynchronous COPY operations being performed in parallel on
> the same file by the server. Therefore, a copy offload stateid with
> a seqid of zero MUST be considered invalid.
>
> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
> Signed-off-by: Dai Ngo <[email protected]>

Applied for the next merge window. See the cel-next topic branch in
this repo:

git://git.linux-nfs.org/projects/cel/cel-2.6.git

See also:

http://git.linux-nfs.org/?p=cel/cel-2.6.git;a=shortlog;h=refs/heads/cel-next


> ---
> fs/nfsd/nfs4state.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d7f27ed6b794..47006eec724e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -769,6 +769,7 @@ static int nfs4_init_cp_state(struct nfsd_net *nn, copy_stateid_t *stid,
> spin_lock(&nn->s2s_cp_lock);
> new_id = idr_alloc_cyclic(&nn->s2s_cp_stateids, stid, 0, 0, GFP_NOWAIT);
> stid->stid.si_opaque.so_id = new_id;
> + stid->stid.si_generation = 1;
> spin_unlock(&nn->s2s_cp_lock);
> idr_preload_end();
> if (new_id < 0)
> --
> 2.9.5
>

--
Chuck Lever