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.
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> Trond.Myklebust@xxxxxxxxxx
> www.netapp.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
--
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
