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

From: Alexander Viro (viro@math.psu.edu)
Date: Fri Nov 30 2001 - 04:57:28 EST


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.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Nov 30 2001 - 21:00:38 EST