Re: Huge race in lockd for async lock requests?

[Tom Talpey <tmtalpey@gmail.com>]

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.

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. Unfortunately, it's also
wrapped up in the BKL use of the VFS locking layer, but in practice those
are two different things. Using the BKL for upcall/downcall synchronization
and lockd thread protection are the issue here.

Tom.