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
Hi Olga,
On Tue, Sep 23, 2014 at 5:36 PM, Olga Kornievskaia
<[email protected]> wrote:
> 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
[email protected]
Thanks, Trond. Will do the changes you suggested and submit another patch.
On Tue, Sep 23, 2014 at 2:51 PM, Trond Myklebust
<[email protected]> wrote:
> Hi Olga,
>
> On Tue, Sep 23, 2014 at 5:36 PM, Olga Kornievskaia
> <[email protected]> wrote:
>> 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
>
> [email protected]