Re: [PATCH 0/4] NFSv4 fix nfs4_stateid_is_current processing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 4, 2014 at 1:09 PM, Trond Myklebust
<trond.myklebust@xxxxxxxxxxxxxxx> wrote:
>
> On Mar 4, 2014, at 12:31, andros@xxxxxxxxxx wrote:
>
>> From: Andy Adamson <andros@xxxxxxxxxx>
>>
>> This is an expanded version of the "NFSv4 always compare stateids in nfs4_stateid_is_current" patch I sent on Feb 27.
>>
>> Found at Connectathon 2014 and NetApp internal testing.
>>
>> nfs4_stateid_is_current is used on the NFSv4 I/O path to determine if a
>> stateid has changed. The idea is that if there is a stateid expire error
>> such as NFS4ERR_BAD_STATEID and the stateid used that induced the error has
>> changed, then we can just resend the RPC from the call prepare state with
>> the changed stateid instead of kicking off recovery as the changed stateid
>> indicates it already had been recovered.
>>
>> This patch set fixes a false positive that resulted in an NFS4ERR_BAD_STATEID
>> infinite loop. Trond pointed out that the nfs4_select_rw_stateid -EIO error
>> is special in that it indicates a lost lock.
>>
>> As I examined the use of nfs4_select_rw_stateid, I addressed some other errors
>> in the use of nfs4_set_rw_stateid and friends in setattr and filelayout
>> prepare functions.
>>
>> Just tested with connectathon tests. Will test more once I'm back in town...
>
> One question:
>
> Do we need the EWOULDBLOCK case at all in nfs4_copy_lock_stateid/nfs4_copy_open_stateid? Looking at the code, it seems particularly redundant for the case of NFSv4.1, where we set the seqid to '0'. Given that we do the nfs4_stateid_is_current() test, it seems unnecessary in the case of NFSv4.0 too...

Yes - Even though you mentioned it's importance at Connectathon, I
could not find a use of the EWOULDBLOCK return.

>
> So, how about just throwing out the EWOULDBLOCK returns, then keeping nfs4_stateid_is_current() as it is,

So, you mean keeping nfs4_stateid_is_current as a boolean, and then
assume that a false  return == -EIO?

I don't like that because that is how this bug was created - mixing
bool and int functions.

> fixing up _nfs4_do_setattr to only deal with the -EIO case,

Yes.

 and then fixing  up the file layout uses of nfs4_select_rw_stateid()
to check the return values?

As I did in the last patch...

>
> _________________________________
> 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
--
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




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux