2019-09-10 08:09:58

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 5/9] NFSv4: Add a helper to increment stateid seqids

Add a helper function to increment stateid seqids according to the
rules specified in RFC5661 Section 8.2.2.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4_fs.h | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 3564da1ba8a1..e8f74ed98e42 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -574,6 +574,15 @@ static inline bool nfs4_stateid_is_newer(const nfs4_stateid *s1, const nfs4_stat
return (s32)(be32_to_cpu(s1->seqid) - be32_to_cpu(s2->seqid)) > 0;
}

+static inline void nfs4_stateid_seqid_inc(nfs4_stateid *s1)
+{
+ u32 seqid = be32_to_cpu(s1->seqid);
+
+ if (++seqid == 0)
+ ++seqid;
+ s1->seqid = cpu_to_be32(seqid);
+}
+
static inline bool nfs4_valid_open_stateid(const struct nfs4_state *state)
{
return test_bit(NFS_STATE_RECOVERY_FAILED, &state->flags) == 0;
--
2.21.0


2019-09-10 08:09:58

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH 6/9] pNFS: Handle NFS4ERR_OLD_STATEID on layoutreturn by bumping the state seqid

If a LAYOUTRETURN receives a reply of NFS4ERR_OLD_STATEID then assume we've
missed an update, and just bump the stateid.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4proc.c | 2 +-
fs/nfs/pnfs.c | 18 ++++++++++++++----
fs/nfs/pnfs.h | 4 ++--
3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a5deb00b5ad1..cbaf6b7ac128 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -9069,7 +9069,7 @@ static void nfs4_layoutreturn_done(struct rpc_task *task, void *calldata)
server = NFS_SERVER(lrp->args.inode);
switch (task->tk_status) {
case -NFS4ERR_OLD_STATEID:
- if (nfs4_layoutreturn_refresh_stateid(&lrp->args.stateid,
+ if (nfs4_layout_refresh_old_stateid(&lrp->args.stateid,
&lrp->args.range,
lrp->args.inode))
goto out_restart;
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index abc7188f1853..bb80034a7661 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -359,9 +359,10 @@ pnfs_clear_lseg_state(struct pnfs_layout_segment *lseg,
}

/*
- * Update the seqid of a layout stateid
+ * Update the seqid of a layout stateid after receiving
+ * NFS4ERR_OLD_STATEID
*/
-bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
+bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
struct pnfs_layout_range *dst_range,
struct inode *inode)
{
@@ -377,7 +378,15 @@ bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,

spin_lock(&inode->i_lock);
lo = NFS_I(inode)->layout;
- if (lo && nfs4_stateid_match_other(dst, &lo->plh_stateid)) {
+ if (lo && pnfs_layout_is_valid(lo) &&
+ nfs4_stateid_match_other(dst, &lo->plh_stateid)) {
+ /* Is our call using the most recent seqid? If so, bump it */
+ if (!nfs4_stateid_is_newer(&lo->plh_stateid, dst)) {
+ nfs4_stateid_seqid_inc(dst);
+ ret = true;
+ goto out;
+ }
+ /* Try to update the seqid to the most recent */
err = pnfs_mark_matching_lsegs_return(lo, &head, &range, 0);
if (err != -EBUSY) {
dst->seqid = lo->plh_stateid.seqid;
@@ -385,6 +394,7 @@ bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
ret = true;
}
}
+out:
spin_unlock(&inode->i_lock);
pnfs_free_lseg_list(&head);
return ret;
@@ -1475,7 +1485,7 @@ int pnfs_roc_done(struct rpc_task *task, struct inode *inode,
*ret = -NFS4ERR_NOMATCHING_LAYOUT;
return 0;
case -NFS4ERR_OLD_STATEID:
- if (!nfs4_layoutreturn_refresh_stateid(&arg->stateid,
+ if (!nfs4_layout_refresh_old_stateid(&arg->stateid,
&arg->range, inode))
break;
*ret = -NFS4ERR_NOMATCHING_LAYOUT;
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 3ef3756d437c..f8a38065c7e4 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -261,7 +261,7 @@ int pnfs_destroy_layouts_byfsid(struct nfs_client *clp,
bool is_recall);
int pnfs_destroy_layouts_byclid(struct nfs_client *clp,
bool is_recall);
-bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
+bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
struct pnfs_layout_range *dst_range,
struct inode *inode);
void pnfs_put_layout_hdr(struct pnfs_layout_hdr *lo);
@@ -798,7 +798,7 @@ static inline void nfs4_pnfs_v3_ds_connect_unload(void)
{
}

-static inline bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
+static inline bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
struct pnfs_layout_range *dst_range,
struct inode *inode)
{
--
2.21.0

2019-09-12 15:18:11

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH 6/9] pNFS: Handle NFS4ERR_OLD_STATEID on layoutreturn by bumping the state seqid

Hi Trond,

Can you explain why are we just bumping the seqid by 1 instead of what
it was currently using to update it to the current value which could
be more than off by one? I'm just speculating that we'll see the same
behavior that we'll get the ERR_OLD_STATEID incremented by one until
the current value?

On Tue, Sep 10, 2019 at 4:09 AM Trond Myklebust <[email protected]> wrote:
>
> If a LAYOUTRETURN receives a reply of NFS4ERR_OLD_STATEID then assume we've
> missed an update, and just bump the stateid.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 2 +-
> fs/nfs/pnfs.c | 18 ++++++++++++++----
> fs/nfs/pnfs.h | 4 ++--
> 3 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index a5deb00b5ad1..cbaf6b7ac128 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -9069,7 +9069,7 @@ static void nfs4_layoutreturn_done(struct rpc_task *task, void *calldata)
> server = NFS_SERVER(lrp->args.inode);
> switch (task->tk_status) {
> case -NFS4ERR_OLD_STATEID:
> - if (nfs4_layoutreturn_refresh_stateid(&lrp->args.stateid,
> + if (nfs4_layout_refresh_old_stateid(&lrp->args.stateid,
> &lrp->args.range,
> lrp->args.inode))
> goto out_restart;
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index abc7188f1853..bb80034a7661 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -359,9 +359,10 @@ pnfs_clear_lseg_state(struct pnfs_layout_segment *lseg,
> }
>
> /*
> - * Update the seqid of a layout stateid
> + * Update the seqid of a layout stateid after receiving
> + * NFS4ERR_OLD_STATEID
> */
> -bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
> +bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
> struct pnfs_layout_range *dst_range,
> struct inode *inode)
> {
> @@ -377,7 +378,15 @@ bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
>
> spin_lock(&inode->i_lock);
> lo = NFS_I(inode)->layout;
> - if (lo && nfs4_stateid_match_other(dst, &lo->plh_stateid)) {
> + if (lo && pnfs_layout_is_valid(lo) &&
> + nfs4_stateid_match_other(dst, &lo->plh_stateid)) {
> + /* Is our call using the most recent seqid? If so, bump it */
> + if (!nfs4_stateid_is_newer(&lo->plh_stateid, dst)) {
> + nfs4_stateid_seqid_inc(dst);
> + ret = true;
> + goto out;
> + }
> + /* Try to update the seqid to the most recent */
> err = pnfs_mark_matching_lsegs_return(lo, &head, &range, 0);
> if (err != -EBUSY) {
> dst->seqid = lo->plh_stateid.seqid;
> @@ -385,6 +394,7 @@ bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
> ret = true;
> }
> }
> +out:
> spin_unlock(&inode->i_lock);
> pnfs_free_lseg_list(&head);
> return ret;
> @@ -1475,7 +1485,7 @@ int pnfs_roc_done(struct rpc_task *task, struct inode *inode,
> *ret = -NFS4ERR_NOMATCHING_LAYOUT;
> return 0;
> case -NFS4ERR_OLD_STATEID:
> - if (!nfs4_layoutreturn_refresh_stateid(&arg->stateid,
> + if (!nfs4_layout_refresh_old_stateid(&arg->stateid,
> &arg->range, inode))
> break;
> *ret = -NFS4ERR_NOMATCHING_LAYOUT;
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 3ef3756d437c..f8a38065c7e4 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -261,7 +261,7 @@ int pnfs_destroy_layouts_byfsid(struct nfs_client *clp,
> bool is_recall);
> int pnfs_destroy_layouts_byclid(struct nfs_client *clp,
> bool is_recall);
> -bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
> +bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
> struct pnfs_layout_range *dst_range,
> struct inode *inode);
> void pnfs_put_layout_hdr(struct pnfs_layout_hdr *lo);
> @@ -798,7 +798,7 @@ static inline void nfs4_pnfs_v3_ds_connect_unload(void)
> {
> }
>
> -static inline bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
> +static inline bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
> struct pnfs_layout_range *dst_range,
> struct inode *inode)
> {
> --
> 2.21.0
>

2019-09-12 18:33:42

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 6/9] pNFS: Handle NFS4ERR_OLD_STATEID on layoutreturn by bumping the state seqid

On Thu, 2019-09-12 at 11:14 -0400, Olga Kornievskaia wrote:
> Hi Trond,
>
> Can you explain why are we just bumping the seqid by 1 instead of
> what
> it was currently using to update it to the current value which could
> be more than off by one? I'm just speculating that we'll see the same
> behavior that we'll get the ERR_OLD_STATEID incremented by one until
> the current value?

We only bump by 1 if we see that we're already above the seqid of the
current layout stateid. Otherwise, we sync to the value of that layout
stateid before retrying.

>
> On Tue, Sep 10, 2019 at 4:09 AM Trond Myklebust <[email protected]>
> wrote:
> > If a LAYOUTRETURN receives a reply of NFS4ERR_OLD_STATEID then
> > assume we've
> > missed an update, and just bump the stateid.
> >
> > Signed-off-by: Trond Myklebust <[email protected]>
> > ---
> > fs/nfs/nfs4proc.c | 2 +-
> > fs/nfs/pnfs.c | 18 ++++++++++++++----
> > fs/nfs/pnfs.h | 4 ++--
> > 3 files changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index a5deb00b5ad1..cbaf6b7ac128 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -9069,7 +9069,7 @@ static void nfs4_layoutreturn_done(struct
> > rpc_task *task, void *calldata)
> > server = NFS_SERVER(lrp->args.inode);
> > switch (task->tk_status) {
> > case -NFS4ERR_OLD_STATEID:
> > - if (nfs4_layoutreturn_refresh_stateid(&lrp-
> > >args.stateid,
> > + if (nfs4_layout_refresh_old_stateid(&lrp-
> > >args.stateid,
> > &lrp->args.range,
> > lrp->args.inode))
> > goto out_restart;
> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> > index abc7188f1853..bb80034a7661 100644
> > --- a/fs/nfs/pnfs.c
> > +++ b/fs/nfs/pnfs.c
> > @@ -359,9 +359,10 @@ pnfs_clear_lseg_state(struct
> > pnfs_layout_segment *lseg,
> > }
> >
> > /*
> > - * Update the seqid of a layout stateid
> > + * Update the seqid of a layout stateid after receiving
> > + * NFS4ERR_OLD_STATEID
> > */
> > -bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
> > +bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
> > struct pnfs_layout_range *dst_range,
> > struct inode *inode)
> > {
> > @@ -377,7 +378,15 @@ bool
> > nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
> >
> > spin_lock(&inode->i_lock);
> > lo = NFS_I(inode)->layout;
> > - if (lo && nfs4_stateid_match_other(dst, &lo->plh_stateid))
> > {
> > + if (lo && pnfs_layout_is_valid(lo) &&
> > + nfs4_stateid_match_other(dst, &lo->plh_stateid)) {
> > + /* Is our call using the most recent seqid? If so,
> > bump it */
> > + if (!nfs4_stateid_is_newer(&lo->plh_stateid, dst))
> > {
> > + nfs4_stateid_seqid_inc(dst);
> > + ret = true;
> > + goto out;
> > + }
> > + /* Try to update the seqid to the most recent */
> > err = pnfs_mark_matching_lsegs_return(lo, &head,
> > &range, 0);
> > if (err != -EBUSY) {
> > dst->seqid = lo->plh_stateid.seqid;
> > @@ -385,6 +394,7 @@ bool
> > nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
> > ret = true;
> > }
> > }
> > +out:
> > spin_unlock(&inode->i_lock);
> > pnfs_free_lseg_list(&head);
> > return ret;
> > @@ -1475,7 +1485,7 @@ int pnfs_roc_done(struct rpc_task *task,
> > struct inode *inode,
> > *ret = -NFS4ERR_NOMATCHING_LAYOUT;
> > return 0;
> > case -NFS4ERR_OLD_STATEID:
> > - if (!nfs4_layoutreturn_refresh_stateid(&arg-
> > >stateid,
> > + if (!nfs4_layout_refresh_old_stateid(&arg->stateid,
> > &arg->range, inode))
> > break;
> > *ret = -NFS4ERR_NOMATCHING_LAYOUT;
> > diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> > index 3ef3756d437c..f8a38065c7e4 100644
> > --- a/fs/nfs/pnfs.h
> > +++ b/fs/nfs/pnfs.h
> > @@ -261,7 +261,7 @@ int pnfs_destroy_layouts_byfsid(struct
> > nfs_client *clp,
> > bool is_recall);
> > int pnfs_destroy_layouts_byclid(struct nfs_client *clp,
> > bool is_recall);
> > -bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid *dst,
> > +bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
> > struct pnfs_layout_range *dst_range,
> > struct inode *inode);
> > void pnfs_put_layout_hdr(struct pnfs_layout_hdr *lo);
> > @@ -798,7 +798,7 @@ static inline void
> > nfs4_pnfs_v3_ds_connect_unload(void)
> > {
> > }
> >
> > -static inline bool nfs4_layoutreturn_refresh_stateid(nfs4_stateid
> > *dst,
> > +static inline bool nfs4_layout_refresh_old_stateid(nfs4_stateid
> > *dst,
> > struct pnfs_layout_range *dst_range,
> > struct inode *inode)
> > {
> > --
> > 2.21.0
> >
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]