> back to Alexander Viro:
> > In other words, patch is completely bogus.
> No, not completely. In a lot of cases we just replaced some regular
> arithmetic with atomic instructions of some sort. These changes are
> still completely valid. But, in the cases where we added locking, we
Valid, but not necessary a good idea. Notice that there are very
real races in ->release() and ->open() instances. Removing BKL
from these (or replacing it with atomic operations) doesn't fix the
existing race. Moreover, it creates a false impression that thing
had been reviewed and fixed.
Well, but for the revelation that the BKL is held in chrdev_open(),
offering implicit serialization for all open routines, these *were*
reviewed and thought fixed. Atomic operations *do* fill the bill when
we are looking at counts used to guarantee exclusive opens. They
generally *don't* fill the bill when you want to take virtually any
other action based on those counts.
Sorry for "told you so", but that's what I'd been trying to explain
from the very beginning - back when the idea of that patch had been
discussed the first time. That work really has to be done
driver-by-driver...
It actually *was* in this case. This may look like an afternoon of
editor exercise, but each of the cases was inspected and reviewed
before recommending the patch, and collectively represents several
weeks' worth of work. In early November I posted a note to
linux-kernel which said, in part:
Before we post these patches, I thought I'd ask if in the time
since Al Viro moved this out here (July 2000) if anybody
(especially him!) has found a *legitimate* use of the BKL in the
release() functions. (We have not found one.)
There was no response.
Given the miss on chrdev_open(), we've got some patch patches to come
up with. However, this isn't a dump-and-run job. We accept the
responsibility for those patches, and we'll patch 'em up. While we
can't then miraculously announce it's safe to remove lock_kernel around
opens, we CAN announce we are closer, and nothing else will be any more
broken than it was before we started. As I said in an earlier post (and
I don't think anybody disagrees): this is an incremental task. Any
assistance you can provide in reviewing this work, including all to
date, is welcome.
Rick
-
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:41 EST