Re: Huge race in lockd for async lock requests?

J. Bruce Fields
2009-05-29T19:14:59Z

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