On Wed, Sep 24, 2014 at 4:11 PM, Trond Myklebust
<trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> On Wed, Sep 24, 2014 at 2:29 PM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote:
>> 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.
>
Ok. Another one submitted.
>> On Wed, Sep 24, 2014 at 2:05 PM, Trond Myklebust
>> <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
>>> Hi Olga,
>>>
>>> On Wed, Sep 24, 2014 at 12:08 PM, Olga Kornievskaia <kolga@xxxxxxxxxx> 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 <kolga@xxxxxxxxxx>
>>>> ---
>>>> 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@xxxxxxxxxxxxxxx
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Trond Myklebust
>
> Linux NFS client maintainer, PrimaryData
>
> trond.myklebust@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
