On Wed, Nov 5, 2014 at 1:38 PM, Trond Myklebust
<trond.myklebust@xxxxxxxxxxxxxxx> wrote:
> On Wed, Nov 5, 2014 at 12:53 PM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote:
>> Thank you for looking into this!
>>
>> On Tue, Nov 4, 2014 at 7:14 PM, Trond Myklebust
>> <trond.myklebust@xxxxxxxxxxxxxxx> wrote:
>>> On Tue, Nov 4, 2014 at 6:22 PM, Olga Kornievskaia <aglo@xxxxxxxxx> wrote:
>>>> Trond, can you please respond to the patch?
>>>>
>>>> As per earlier conversation, in this solution, state recovery is
>>>> initiated which marks the locks lost.
>>>>
>>>> Please either accept this patch or let me know what needs to be fixed.
>>>>
>>>
>>> Please see the 3 fixes I just sent out concerning delegation recovery
>>> w.r.t. NFSv4+NFSv4.1. In addition, we need to handle the case you
>>> patch attempts to address (however see the question I have below).
>>>
>>>> Thank you.
>>>>
>>>> On Mon, Nov 3, 2014 at 9:36 AM, Olga Kornievskaia <kolga@xxxxxxxxxx> wrote:
>>>>> If we get a bad-stateid-type of error when we send OPEN with delegate_cur
>>>>> to return currently held delegation, we shouldn't be trying to reclaim locks
>>>>> associated with that delegation state_id because we don't have an
>>>>> open_stateid to be used for the LOCK operation. Thus, we should
>>>>> return an error from the nfs4_open_delegation_recall() in that case.
>>>>>
>>>>> Furthermore, if an error occurs the delegation code will call
>>>>> nfs_abort_delegation_return() which sets again the NFS4CLNT_DELEGRETURN
>>>>> flags in the state and it leads the state manager to into an infinite loop
>>>>> for trying to reclaim the delegated state.
>>>>>
>>>>> Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
>>>>> ---
>>>>> fs/nfs/delegation.c | 5 +++--
>>>>> fs/nfs/nfs4proc.c | 2 +-
>>>>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
>>>>> index 5853f53..8016d89 100644
>>>>> --- a/fs/nfs/delegation.c
>>>>> +++ b/fs/nfs/delegation.c
>>>>> @@ -394,7 +394,7 @@ static int nfs_end_delegation_return(struct inode *inode, struct nfs_delegation
>>>>> err = nfs4_wait_clnt_recover(clp);
>>>>> } while (err == 0);
>>>>>
>>>>> - if (err) {
>>>>> + if (err && err != -EIO) {
>>>>> nfs_abort_delegation_return(delegation, clp);
>>>
>>> This exception for EIO now has me worried. If we detach the
>>> delegation, then it looks to me as if we will never send a
>>> FREE_STATEID, as required for the case of NFSv4.1.
>>
>> I'd need to brush up on the FREE_STATEID op to confidently answer
>> that. But can't this be taken care of in state_recovery when locks are
>> marked lost?
>
> No, because in the case of EIO we now fall through to
> nfs_detach_delegation() and nfs_do_return_delegation(). What we really
> want to do is call TEST_STATEID + FREE_STATEID (see RFC5661 sections
> 8.2.4, 8.5 and 18.38.3), which will now be done as part of state
> recovery.
I don't see it. :-/ Is it because there is a race between returning a
delegation and state recovery that was initiated? Because as far as I
can see, the stateid recovery initiated by the
handle_delegation_recall_error() will call nfs4_test_stateid() via
nfs41_open_expire().
>
> --
> 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
