[PATCH 1/1] Fixing lease renewal

Discussion:

Olga Kornievskaia

2014-09-23 21:36:05 UTC

Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.

---
fs/nfs/nfs4state.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..790aed3 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2345,6 +2345,8 @@ static void nfs4_state_manager(struct nfs_client *clp)
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state))
+ continue;
}

if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Trond Myklebust

2014-09-23 21:51:11 UTC

Permalink

Hi Olga,

On Tue, Sep 23, 2014 at 5:36 PM, Olga Kornievskaia

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..790aed3 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2345,6 +2345,8 @@ static void nfs4_state_manager(struct nfs_client *clp)
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state))
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {

What say we just move the "if (test_and_clear_bit(NFS4CLNT_MOVED,
&clp->cl_state)) { ... }" section so that it comes immediately before
the NFS4CLNT_CHECK_LEASE test?
That way we can just reinstate the original 'continue' without the
extra test above, and ensure that the NFS4CLNT_MOVED is always handled
before trying the check lease.

Also, please do remember to add a "Signed-off-by:" line when submitting patches.

Thanks
Trond
--
Trond Myklebust

Linux NFS client maintainer, PrimaryData

trond.myklebust-7I+n7zu2hftEKMMhf/***@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Olga Kornievskaia

2014-09-24 13:10:35 UTC

Permalink

Thanks, Trond. Will do the changes you suggested and submit another patch.

On Tue, Sep 23, 2014 at 2:51 PM, Trond Myklebust

Post by Trond Myklebust
Hi Olga,
On Tue, Sep 23, 2014 at 5:36 PM, Olga Kornievskaia

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Olga Kornievskaia

2014-09-24 13:11:04 UTC

Permalink

Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.

Signed-off-by: Olga Kornievskaia <kolga-HgOvQuBEEgTQT0dZR+***@public.gmane.org>
---
fs/nfs/nfs4state.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}

- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}

if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {

--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Anna Schumaker

2014-09-24 13:46:54 UTC

Permalink

Hey Olga,

Your patch is using spaces instead of tabs, so it won't apply to the existing code. Did you use `git format-patch` to generate it?

Anna

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}
- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Olga Kornievskaia

2014-09-24 15:08:27 UTC

Permalink

I did. Let me try again.

On Wed, Sep 24, 2014 at 9:46 AM, Anna Schumaker

Post by Anna Schumaker
Hey Olga,
Your patch is using spaces instead of tabs, so it won't apply to the existing code. Did you use `git format-patch` to generate it?
Anna

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}
- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Olga Kornievskaia

2014-09-24 16:08:31 UTC

Permalink

Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.

Signed-off-by: Olga Kornievskaia <kolga-HgOvQuBEEgTQT0dZR+***@public.gmane.org>
---
fs/nfs/nfs4state.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}

- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}

if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {

--
1.8.5.2 (Apple Git-48)

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Trond Myklebust

2014-09-24 18:05:53 UTC

Permalink

Hi Olga,

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}
- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
--
1.8.5.2 (Apple Git-48)

I think you misunderstood me. I was proposing that we move the entire
code section that currently reads

if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
section = "migration";
status = nfs4_handle_migration(clp);
if (status < 0)
goto out_error;
}

so that it occurs before the section

if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}

That way we only need to test once for NFS4CLNT_MOVED.

Cheers
Trond
--
Trond Myklebust

Linux NFS client maintainer, PrimaryData

trond.myklebust-7I+n7zu2hftEKMMhf/***@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Olga Kornievskaia

2014-09-24 18:21:09 UTC

Permalink

I have misunderstood you. One more try coming up.

On Wed, Sep 24, 2014 at 2:05 PM, Trond Myklebust

Post by Trond Myklebust
Hi Olga,

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}
- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
--
1.8.5.2 (Apple Git-48)

I think you misunderstood me. I was proposing that we move the entire
code section that currently reads
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
section = "migration";
status = nfs4_handle_migration(clp);
if (status < 0)
goto out_error;
}
so that it occurs before the section
if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
That way we only need to test once for NFS4CLNT_MOVED.
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Olga Kornievskaia

2014-09-24 18:29:16 UTC

Permalink

Before I go ahead with the change, does it make sense to move "both"
of the migration sections NFS4CLNT_MOVED and NFS4CLNT_LEASE_MOVED
before the check lease section?

On Wed, Sep 24, 2014 at 2:05 PM, Trond Myklebust

Post by Trond Myklebust
Hi Olga,

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}
- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
--
1.8.5.2 (Apple Git-48)

I think you misunderstood me. I was proposing that we move the entire
code section that currently reads
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
section = "migration";
status = nfs4_handle_migration(clp);
if (status < 0)
goto out_error;
}
so that it occurs before the section
if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
That way we only need to test once for NFS4CLNT_MOVED.
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Trond Myklebust

2014-09-24 20:11:39 UTC

Permalink

Post by Olga Kornievskaia
Before I go ahead with the change, does it make sense to move "both"
of the migration sections NFS4CLNT_MOVED and NFS4CLNT_LEASE_MOVED
before the check lease section?

Actually. Never mind. Thinking about this, and looking at the code, we
definitely do want to ensure that lease recovery is performed before
migration recovery.
Let's just put the 'continue;' back into the CHECK_LEASE section for
now so that we enforce that.

Post by Olga Kornievskaia
On Wed, Sep 24, 2014 at 2:05 PM, Trond Myklebust

Post by Trond Myklebust
Hi Olga,

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}
- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
--
1.8.5.2 (Apple Git-48)

I think you misunderstood me. I was proposing that we move the entire
code section that currently reads
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
section = "migration";
status = nfs4_handle_migration(clp);
if (status < 0)
goto out_error;
}
so that it occurs before the section
if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
That way we only need to test once for NFS4CLNT_MOVED.
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Trond Myklebust

Linux NFS client maintainer, PrimaryData

trond.myklebust-7I+n7zu2hftEKMMhf/***@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Olga Kornievskaia

2014-09-24 22:12:32 UTC

Permalink

On Wed, Sep 24, 2014 at 4:11 PM, Trond Myklebust

Post by Trond Myklebust

Post by Olga Kornievskaia
Before I go ahead with the change, does it make sense to move "both"
of the migration sections NFS4CLNT_MOVED and NFS4CLNT_LEASE_MOVED
before the check lease section?

Ok. Another one submitted.

Post by Trond Myklebust

Post by Olga Kornievskaia
On Wed, Sep 24, 2014 at 2:05 PM, Trond Myklebust

Post by Trond Myklebust
Hi Olga,

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..4616598 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2340,11 +2340,13 @@ static void nfs4_state_manager(struct nfs_client *clp)
continue;
}
- if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
+ if (!test_bit(NFS4CLNT_MOVED, &clp->cl_state) &&
+ test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
--
1.8.5.2 (Apple Git-48)

I think you misunderstood me. I was proposing that we move the entire
code section that currently reads
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
section = "migration";
status = nfs4_handle_migration(clp);
if (status < 0)
goto out_error;
}
so that it occurs before the section
if (test_and_clear_bit(NFS4CLNT_CHECK_LEASE, &clp->cl_state)) {
section = "check lease";
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
That way we only need to test once for NFS4CLNT_MOVED.
Cheers
Trond
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Trond Myklebust
Linux NFS client maintainer, PrimaryData

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Olga Kornievskaia

2014-09-24 22:11:28 UTC

Permalink

Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.

Signed-off-by: Olga Kornievskaia <kolga-HgOvQuBEEgTQT0dZR+***@public.gmane.org>
---
fs/nfs/nfs4state.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..6678769 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2345,6 +2345,7 @@ static void nfs4_state_manager(struct nfs_client *clp)
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}

if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {

Trond Myklebust

2014-09-24 22:24:05 UTC

Permalink

Post by Olga Kornievskaia
Commit c9fdeb28 removed a 'continue' after checking if the lease needs
to be renewed. However, if client hasn't moved, the code falls down to
starting reboot recovery erroneously (ie., sends open reclaim and gets
back stale_clientid error) before recovering from getting stale_clientid
on the renew operation.
---
fs/nfs/nfs4state.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 22fe351..6678769 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -2345,6 +2345,7 @@ static void nfs4_state_manager(struct nfs_client *clp)
status = nfs4_check_lease(clp);
if (status < 0)
goto out_error;
+ continue;
}
if (test_and_clear_bit(NFS4CLNT_MOVED, &clp->cl_state)) {
--
1.8.5.2 (Apple Git-48)

Thanks Olga! Will apply.
--
Trond Myklebust

Linux NFS client maintainer, PrimaryData

trond.myklebust-7I+n7zu2hftEKMMhf/***@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html