Re: Live lock in silly-rename.

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

 



On Wed, Jun 4, 2014 at 3:39 AM, NeilBrown <neilb@xxxxxxx> wrote:
> On Sat, 31 May 2014 08:13:58 +1000 NeilBrown <neilb@xxxxxxx> wrote:
>
>> On Fri, 30 May 2014 17:55:23 -0400 "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
>> wrote:
>>
>> > On Fri, May 30, 2014 at 01:44:42PM +1000, NeilBrown wrote:
>> > > On Thu, 29 May 2014 20:44:23 -0400 "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
>> > > wrote:
>> > >
>> > > > Yes, it's a known server bug.
>> > > >
>> > > > As a first attempt I was thinking of just sticking a timestamp in struct
>> > > > inode to record the time of the most recent conflicting access and deny
>> > > > delegations if the timestamp is too recent, for some definition of too
>> > > > recent.
>> > > >
>> > >
>> > > Hmmm... I'll have a look next week and see what I can come up with.
>> >
>> > Thanks!
>> >
>> > If we didn't think it was worth another struct inode field, we could
>> > probably get away with global state.  Even just refusing to give out any
>> > delegations for a few seconds after any delegation break would be enough
>> > to fix this bug.
>> >
>> > Or you could make it a little less harsh with a small hash table: "don't
>> > give out a delegation on any inode whose inode number hashes to X for a
>> > few seconds."
>>
>> I was thinking of using a bloom filter - or possibly two.
>> - avoid handing out delegations if either bloom filter reports a match
>> - when reclaiming a delegation add the inode to the second bloom filter
>> - every so-often zero-out the older filter and swap them.
>>
>> Might be a bit of overkill, but I won't know until I implement it.
>>
>
> Below is my suggestion.  It seems easy enough.  It even works.
>
> However it does raise an issue with the NFS client.
>
> NFS performs a silly-rename as an 'asynchronous' operation.  One consequence
> of this is that NFS4ERR_DELAY always results in a delay of
> NFS4_POLL_RETRY_MAX (15*HZ), where as sync requests use an exponential scale
> from _MIN to _MAX.
>
> So in my test case there is always a 15second delay:
>   - try to silly-rename
>   - get NFS4ERR_DELAY
>   - server reclaim delegation
>   - 15 seconds passes
>   - retry silly-rename - it works.
>
> I hacked the NFS server to store a timeout in 'struct nfs_renamedata', and
> use the same exponential retry pattern and the 15 seconds (obviously)
> disappeared.
>
> Trond: would  you accept a patch which did that more generally?  e.g. pass a
> timeout pointer to nfs4_async_handle_error() and various *_done function pass
> a pointer to a field in their calldata?

It depends. If we're touching nfs4_async_handle_error, then I think we
should also convert nfs4_async_handle_error to use the same "struct
nfs4_exception" argument that we use for the synchronous case so that
we can share a bit more code.

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




[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