On Jul 9, 2012, at 4:15 PM, Chuck Lever wrote:
>
> On Jul 9, 2012, at 4:03 PM, Myklebust, Trond wrote:
>
>> On Mon, 2012-07-09 at 11:44 -0400, Chuck Lever wrote:
>>> Before beginning state recovery, the state manager zaps open and lock
>>> state for each open file, thus the "state->flags & flags" test in
>>> nfs41_{open,lock}_expired() always fails during reclaim. But open
>>> recovery is still needed for these files.
>>>
>>> To force a call to nfs4_open_expired(), change the default return
>>> value for nfs41_check_expired_stateid() to force open recovery, and
>>> the default return value for nfs41_check_locks() to force lock
>>> recovery, if the requested flags are clear. Fix suggested by Bryan
>>> Schumaker.
>>
>> This makes absolutely no sense...
>>
>> The point of these function should be to test if the stateid is still
>> valid. If it is, then we need no recovery.
>> If the stateid is _not_ valid, then we free it, and then decide if
>> recovery is needed. The only exception to that rule is if TEST_STATEID
>> returns NFS4ERR_BAD_STATEID, (in which case we don't need to free the
>> stateid but just recover immediately).
>
> So you'd like to change the sense of the switch statement in my earlier patch:
>
> NFS4_OK:
> do nothing
> NFS4ERR_BAD_STATEID:
> just open
> default:
> free the state ID, then open
>
> I still don't think we need a FREE_STATEID call for NFS4ERR_EXPIRED, for example. It seems to me the only cases where FREE_STATEID is needed is the _REVOKED cases.
>
>> IOW: The state->flags and lsp->ls_flags tests should just be removed.
>
> Bryan had a reason to leave those tests in, but I don't remember what it was.
Ah.
I think it was that if those flags are set, then the client assumes the server still has that state, thus no recovery actions should be done.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
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
