Re: [PATCH] remove BKL from drivers' release functions

[Alexander Viro]

On Wed, 28 Nov 2001, David C. Hansen wrote:

> Alan Cox wrote:
> 
> >The release function was only partially serialized - what stops a release
> >and an open racing each other ? That in several cases was why the lock
> >was there. 

->release() is not serialized AT ALL.  It is serialized for given struct file,
but call open(2) twice and you've got two struct file for the same device.
close() both and you've got two calls of ->release(), quite possibly -
simultaneous.

> Nothing, because the BKL is not held for all opens anymore.  In most of 

RTFS.  fs/devices.c::chrdev_open().

> the cases that we addressed, the BKL was in release _only_, not in open 
> at all.  There were quite a few drivers where we added a spinlock, or 

See above.

> used atomic operations to keep open from racing with release.  

Right.  Including quite a few where you schedule under that spinlock.

In other words, patch is completely bogus.  BKL removal may be a good
idea, but you really need to audit the code.  Which requires at least
some understanding of the things you are doing.  There _are_ races
and they need to be dealt with.  But blind BKL removal doesn't fix any
and breaks quite a few places where the code was actually correct.