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 <[email protected]>
---
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)
Hi Olga,
On Wed, Sep 24, 2014 at 12:08 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.
>
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> 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
[email protected]
I have misunderstood you. One more try coming up.
On Wed, Sep 24, 2014 at 2:05 PM, Trond Myklebust
<[email protected]> wrote:
> Hi Olga,
>
> On Wed, Sep 24, 2014 at 12:08 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.
>>
>> Signed-off-by: Olga Kornievskaia <[email protected]>
>> ---
>> 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
>
> [email protected]
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html