From bd25f4dd6972755579d0ea50d1a5ace2e9b00d1a Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Sun, 11 Jul 2010 15:34:05 +0200 Subject: [PATCH] HID: hiddev: use usb_find_interface, get rid of BKL This removes the private hiddev_table in the usbhid driver and changes it to use usb_find_interface instead. The advantage is that we can avoid the race between usb_register_dev and usb_open and no longer need the big kernel lock. This doesn't introduce race condition -- the intf pointer could be invalidated only in hiddev_disconnect() through usb_deregister_dev(), but that will block on minor_rwsem and not actually remove the device until usb_open(). Signed-off-by: Arnd Bergmann Cc: Jiri Kosina Cc: "Greg Kroah-Hartman" Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hiddev.c | 54 +++++++++---------------------------- 1 file changed, 12 insertions(+), 42 deletions(-) diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c index c24d2fa3e3b6..254a003af048 100644 --- a/drivers/hid/usbhid/hiddev.c +++ b/drivers/hid/usbhid/hiddev.c @@ -67,7 +67,7 @@ struct hiddev_list { struct mutex thread_lock; }; -static struct hiddev *hiddev_table[HIDDEV_MINORS]; +static struct usb_driver hiddev_driver; /* * Find a report, given the report's type and ID. The ID can be specified @@ -265,22 +265,19 @@ static int hiddev_release(struct inode * inode, struct file * file) static int hiddev_open(struct inode *inode, struct file *file) { struct hiddev_list *list; - int res, i; - - /* See comment in hiddev_connect() for BKL explanation */ - lock_kernel(); - i = iminor(inode) - HIDDEV_MINOR_BASE; + struct usb_interface *intf; + struct hiddev *hiddev; + int res; - if (i >= HIDDEV_MINORS || i < 0 || !hiddev_table[i]) + intf = usb_find_interface(&hiddev_driver, iminor(inode)); + if (!intf) return -ENODEV; + hiddev = usb_get_intfdata(intf); if (!(list = kzalloc(sizeof(struct hiddev_list), GFP_KERNEL))) return -ENOMEM; mutex_init(&list->thread_lock); - - list->hiddev = hiddev_table[i]; - - + list->hiddev = hiddev; file->private_data = list; /* @@ -289,7 +286,7 @@ static int hiddev_open(struct inode *inode, struct file *file) */ if (list->hiddev->exist) { if (!list->hiddev->open++) { - res = usbhid_open(hiddev_table[i]->hid); + res = usbhid_open(hiddev->hid); if (res < 0) { res = -EIO; goto bail; @@ -301,12 +298,12 @@ static int hiddev_open(struct inode *inode, struct file *file) } spin_lock_irq(&list->hiddev->list_lock); - list_add_tail(&list->node, &hiddev_table[i]->list); + list_add_tail(&list->node, &hiddev->list); spin_unlock_irq(&list->hiddev->list_lock); if (!list->hiddev->open++) if (list->hiddev->exist) { - struct hid_device *hid = hiddev_table[i]->hid; + struct hid_device *hid = hiddev->hid; res = usbhid_get_power(hid); if (res < 0) { res = -EIO; @@ -314,13 +311,10 @@ static int hiddev_open(struct inode *inode, struct file *file) } usbhid_open(hid); } - - unlock_kernel(); return 0; bail: file->private_data = NULL; kfree(list); - unlock_kernel(); return res; } @@ -894,37 +888,14 @@ int hiddev_connect(struct hid_device *hid, unsigned int force) hid->hiddev = hiddev; hiddev->hid = hid; hiddev->exist = 1; - - /* - * BKL here is used to avoid race after usb_register_dev(). - * Once the device node has been created, open() could happen on it. - * The code below will then fail, as hiddev_table hasn't been - * updated. - * - * The obvious fix -- introducing mutex to guard hiddev_table[] - * doesn't work, as usb_open() and usb_register_dev() both take - * minor_rwsem, thus we'll have ABBA deadlock. - * - * Before BKL pushdown, usb_open() had been acquiring it in right - * order, so _open() was safe to use it to protect from this race. - * Now the order is different, but AB-BA deadlock still doesn't occur - * as BKL is dropped on schedule() (i.e. while sleeping on - * minor_rwsem). Fugly. - */ - lock_kernel(); + usb_set_intfdata(usbhid->intf, usbhid); retval = usb_register_dev(usbhid->intf, &hiddev_class); if (retval) { err_hid("Not able to get a minor for this device."); hid->hiddev = NULL; - unlock_kernel(); kfree(hiddev); return -1; - } else { - hid->minor = usbhid->intf->minor; - hiddev_table[usbhid->intf->minor - HIDDEV_MINOR_BASE] = hiddev; } - unlock_kernel(); - return 0; } @@ -942,7 +913,6 @@ void hiddev_disconnect(struct hid_device *hid) hiddev->exist = 0; mutex_unlock(&hiddev->existancelock); - hiddev_table[hiddev->hid->minor - HIDDEV_MINOR_BASE] = NULL; usb_deregister_dev(usbhid->intf, &hiddev_class); if (hiddev->open) { -- 2.34.1