[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-nfs
Subject:    Re: Huge race in lockd for async lock requests?
From:       "J. Bruce Fields" <bfields () fieldses ! org>
Date:       2009-05-29 19:14:59
Message-ID: 20090529191459.GI29778 () fieldses ! org
[Download RAW message or body]

On Fri, May 29, 2009 at 09:24:25AM -0600, Rob Gardner wrote:
> Tom Talpey wrote:
>> At 10:59 PM 5/28/2009, Rob Gardner wrote:
>>   
>>> J. Bruce Fields wrote:
>>>     
>>>> Looking at the code....  This is all under the BKL, and as far as I can
>>>> tell there aren't any blocking operations anywhere there, so I don't
>>>> think this should happen if the filesystem is careful.  Have you seen it
>>>> happen?
>>>>       
>>> Aha, I just figured it out and you were right. The filesystem in this 
>>> case was not careful. It broke the rules and actually made the 
>>> fl_grant call *before* even returning to nlmsvc_lock's call to 
>>> vfs_lock_file, and it did it in the lockd thread! So the BKL was of 
>>> no use, and I saw nlmsvc_grant_deferred print "grant for unknown 
>>> block". So I think everything is ok, no huge race in lockd for async 
>>> lock requests. Thank you for clearing this up.
>>>     
>>
>> Gack! I'm surprised it worked at all. The fact that the BKL allows itself to
>> be taken recursively really masked your filesystem bug. If the BKL had
>> blocked, or asserted, the bug would never have happened.
>>   
>
> Yeah, recall that I'm using a very old kernel (circa 2.6.18) which I  
> think must still allow the BKL to be acquired recursively.

That's still true on recent kernels.

--b.

>
>> This is as good a time as any to point out that the BKL's use in the lockd
>> code is insidious and needs some serious attention. 
> No disagreement here! I think I almost understand enough about lockd to  
> remove the BKL, but the operative word there is "almost".
>
>
> Rob Gardner
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]