lkml.org 
[lkml]   [2002]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: BKL removal
    On Sun, Jul 07, 2002 at 10:58:04PM -0400, Alexander Viro wrote:
    > On Mon, 8 Jul 2002, Matthew Wilcox wrote:
    > > one struct file per open(), yes. however, fork() shares a struct file,
    > > as does unix domain fd passing. so we need protection between different
    > > processes. there's some pretty good reasons to want to use a semaphore
    > > to protect the struct file (see fasync code.. bleugh).
    >
    > ??? What exactly do you want to protect there?

    andrea & i discussed this off-list a few days ago... see fs/fcntl.c

    case F_SETFL:
    lock_kernel();
    err = setfl(fd, filp, arg);
    unlock_kernel();

    setfl() does:

    if ((arg ^ filp->f_flags) & FASYNC) {
    if (filp->f_op && filp->f_op->fasync) {
    error = filp->f_op->fasync(fd, filp, (arg & FASYNC) != 0);

    and:

    if (arg & O_DIRECT) {
    down(&inode->i_sem);
    if (!filp->f_iobuf)
    error = alloc_kiovec(1, &filp->f_iobuf);
    up(&inode->i_sem);

    and finally:

    filp->f_flags = (arg & SETFL_MASK) | (filp->f_flags & ~SETFL_MASK);

    i pointed out that if alloc_kiovec slept, the BKL provides no protection
    against someone else doing a setfl at the same time, so we can get the
    wrong number of fasync events sent. Marcus Alanen pointed out that
    fasync can also sleep, so we're at risk anyway. i don't think that
    abusing i_sem as andrea did is the Right Thing to do...

    --
    Revolutions do not require corporate support.
    -
    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/

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