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.
>> goto out;
>> }
>> @@ -458,7 +458,8 @@ restart:
>> iput(inode);
>> if (!err)
>> goto restart;
>> - set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state);
>> + if (err != -EIO)
>> + set_bit(NFS4CLNT_DELEGRETURN, &clp->cl_state);
Please explain why this is needed. If we've cleared the bad
delegation, then why should we not attempt to return any others that
may be pending?
>> return err;
>> }
>> }
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 5aa55c1..6871055 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -1655,7 +1655,7 @@ static int nfs4_handle_delegation_recall_error(struct nfs_server *server, struct
>> nfs_inode_find_state_and_recover(state->inode,
>> stateid);
>> nfs4_schedule_stateid_recovery(server, state);
>> - return 0;
>> + return -EIO;
>> case -NFS4ERR_DELAY:
>> case -NFS4ERR_GRACE:
>> set_bit(NFS_DELEGATED_STATE, &state->flags);
>> --
>> 1.7.1
>>
>> --
>> 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
