lkml.org 
[lkml]   [2001]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] Remove needless BKL from release functions
    In article <Pine.SOL.4.33.0111231106530.7403-100000@sun3.lrz-muenchen.de> you wrote:
    > While this is doubtlessly true, please don't do things like removing the
    > lock from interfaces like the call to open() in the input subsystem.
    > People may depend on the lock being held there. Having open() under BKL
    > simplifies writing USB device drivers.

    Beeing completly single-threaded also simplifies writing unclean drivers..

    BTW, I've attached a patch that fixes the largest input races (against 2.4.6),
    I don't see how to change the total lack of locking for other data structures
    without an API change, though.

    Christoph

    --
    Whip me. Beat me. Make me maintain AIX.

    --- linux-2.4.6/drivers/input/input.c Mon Jun 4 21:17:55 2001
    +++ linux/drivers/input/input.c Sun Jul 8 22:58:10 2001
    @@ -57,6 +57,7 @@
    static devfs_handle_t input_devfs_handle;
    static int input_number;
    static long input_devices[NBITS(INPUT_DEVICES)];
    +static DECLARE_MUTEX(input_lock);

    void input_event(struct input_dev *dev, unsigned int type, unsigned int code, int value)
    {
    @@ -222,6 +223,8 @@
    struct input_handler *handler = input_handler;
    struct input_handle *handle;

    + down(&input_lock);
    +
    /*
    * Initialize repeat timer to default values.
    */
    @@ -257,6 +260,8 @@
    input_link_handle(handle);
    handler = handler->next;
    }
    +
    + up(&input_lock);
    }

    void input_unregister_device(struct input_dev *dev)
    @@ -265,6 +270,8 @@
    struct input_dev **devptr = &input_dev;
    struct input_handle *dnext;

    + down(&input_lock);
    +
    /*
    * Kill any pending repeat timers.
    */
    @@ -294,6 +301,8 @@

    if (dev->number < INPUT_DEVICES)
    clear_bit(dev->number, input_devices);
    +
    + up(&input_lock);
    }

    void input_register_handler(struct input_handler *handler)
    @@ -301,6 +310,8 @@
    struct input_dev *dev = input_dev;
    struct input_handle *handle;

    + down(&input_lock);
    +
    /*
    * Add minors if needed.
    */
    @@ -324,6 +335,8 @@
    input_link_handle(handle);
    dev = dev->next;
    }
    +
    + up(&input_lock);
    }

    void input_unregister_handler(struct input_handler *handler)
    @@ -332,6 +345,8 @@
    struct input_handle *handle = handler->handle;
    struct input_handle *hnext;

    + down(&input_lock);
    +
    /*
    * Tell the handler to disconnect from all devices it keeps open.
    */
    @@ -358,38 +373,60 @@

    if (handler->fops != NULL)
    input_table[handler->minor >> 5] = NULL;
    +
    + up(&input_lock);
    }

    static int input_open_file(struct inode *inode, struct file *file)
    {
    - struct input_handler *handler = input_table[MINOR(inode->i_rdev) >> 5];
    struct file_operations *old_fops, *new_fops = NULL;
    + struct input_handler *handler;
    + unsigned int minor = MINOR(inode->i_rdev), index;
    int err;

    - /* No load-on-demand here? */
    - if (!handler || !(new_fops = fops_get(handler->fops)))
    + if (minor >= INPUT_DEVICES)
    return -ENODEV;

    - /*
    - * That's _really_ odd. Usually NULL ->open means "nothing special",
    - * not "no device". Oh, well...
    - */
    - if (!new_fops->open) {
    - fops_put(new_fops);
    - return -ENODEV;
    + down(&input_lock);
    + index = minor >> 5;
    + handler = input_table[index];
    +
    + if (handler)
    + new_fops = fops_get(handler->fops);
    + if (!new_fops) {
    + char modname[32];
    +
    + up(&input_lock);
    + sprintf(modname, "input-handler-%d", index);
    + request_module(modname);
    + down(&input_lock);
    +
    + err = -ENODEV;
    + handler = input_table[index];
    + if (!handler)
    + goto end;
    + if (!(new_fops = fops_get(handler->fops)))
    + goto end;
    }
    +
    old_fops = file->f_op;
    file->f_op = new_fops;

    - lock_kernel();
    - err = new_fops->open(inode, file);
    - unlock_kernel();
    + if (new_fops->open) {
    + lock_kernel();
    + err = new_fops->open(inode, file);
    + unlock_kernel();
    + } else
    + err = 0;

    if (err) {
    fops_put(file->f_op);
    file->f_op = fops_get(old_fops);
    }
    +
    fops_put(old_fops);
    +end:
    + up(&input_lock);
    return err;
    }

    -
    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:13    [W:0.027 / U:29.976 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site