[pnfs] [PATCH] nfsd: use list_del_init in unhash_stateowner

[J. Bruce Fields]

On Sat, Jan 26, 2008 at 10:44:27AM +0200, Benny Halevy wrote:
> J. Bruce Fields wrote:
>> On Thu, Jan 24, 2008 at 02:47:37PM +0200, Benny Halevy wrote:
>>   
>>> If release_stateowner is called more than once
>>> on the same nfs4_stateowner, unhash_stateowner crashes
>>> when calling list_del on a previously poisoned list_head.
>>>     
>>
>> I'm not necessarily opposed to changing these to list_del_init()'s just
>> out of general paranoia, and I'm not doubting the bug at all, but I
>> don't see how it's happening--I've stared at the code and just can't see
>> how release_stateowner() could be getting called twice on the same
>> stateowner.  As far as I can tell, every caller of release_stateowner()
>> gets the state lock, either creates the stateowner or finds it using
>> some data structure, then calls release_stateowner(), which removes the
>> stateowner from every structure that any such callers could use to find
>> it again.
>>   
>
> I'm afraid that this is specific to the pnfs tree where the state lock  
> is being released in nfsd4_close
> around the call to nfsd_close to allow the file system to release state,  
> the layout in particular, that may
> be recalled on close.
> See  
> http://git.linux-nfs.org/?p=bhalevy/linux-pnfs.git;a=commitdiff;h=100222ddc0e03d0e44825717cf485d4f9811701a

OK, that explains it.

So I guess what happened was the client either expired (if the close was
blocked for a lease period without the client renewing?) or the close
got resent?

The latter might have gotten caught if we had the replay cache turned
on.  But this sort of thing is supposed to be caught even without a
replay.

I'd hope a close would normally be something the filesystem can service
instantly--why does it need to wait on layout returns?

But if not then maybe we need something like the RC_INPROG that the rpc
cache has, to indicate that an operation is in progress on a given
stateid?

--b.