[lkml]   [2001]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] remove BKL from drivers' release functions

    On Fri, 30 Nov 2001, David C. Hansen wrote:

    > Alexander Viro wrote:
    > > ->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.
    > OK, that clears some things up. So, the file->fcount is only used in
    > cases where the file descriptor was dup'd, right?

    Think for a minute - current IO position is in file->f_pos. So if two
    descriptors have independent current positions (lseek() on one of them
    doesn't affect another) they have to have different instances of
    struct file behind them.

    struct file is an opened channel
    descriptor refers to opened channel
    file->f_count is the number of references to _this_ channel
    dup() creates a descriptor refering to the same channel
    so does fork()
    open() creates a new channel and descriptor refering to it.

    That difference between descriptors and opened channels exists in any
    UNIX - otherwise you would either have lseek() done in one program
    screwing everybody else or (if fork would create new channels instead
    of new references to channels) have (ls; ls) > foo break (shell opens
    foo and both ls(1) inherit it over fork(); you _really_ want changes
    in current positions resulting from the first ls to affect the
    stdout of the second one).

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

    > need to reevaluate them for potential problem. In the cases where we
    > just removed the BKL, we really need to check them to make sure that we
    > didn't introduce anything.

    You really need that in all cases. 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...

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 13:13    [W:0.024 / U:4.708 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site