On Wednesday 04 Jan 2012, Alan Stern wrote:
> On Wed, 4 Jan 2012, Huajun Li wrote:
> > Hi Alan,
> >
> > Is any chance for user to trigger the window if the driver has been
> >
> > unregistered? thanks.
>
> Yes, that possibility does exist.
>
> > How about adding more codes into usb_store_new_id() to find out
> > whether the driver is registered while receiving such requests? and
> > this could give useful info to user space if the requests fail.
>
> I don't think that's the best approach. Furthermore, your proposed fix
> still contains a race because is_drv_ready() calls put_driver() before
> returning.
>
> In order to fix this properly, we have to make sure that
> usb_store_new_id() is mutually exclusive with driver registration and
> unregistration. The patch below will accomplish that.
>
> However it does leave one problem unsolved. The udev rule still can
> run too quickly. Now instead of getting an oops, the rule simply won't
> work. I don't know how to fix that.
>
> David, can you test this patch in place of the earlier one?
I can try. Building kernels on this elderly box takes hours, and I
had all manner of problems getting to the point of testing the last
patch, but I will give it a go. It will probably be early next week
when I have time to do it (I am busy tomorrow, friday, saturday and
sunday).
David
>
> Alan Stern
>
>
>
> Index: usb-3.2/drivers/usb/core/driver.c
> ===================================================================
> --- usb-3.2.orig/drivers/usb/core/driver.c
> +++ usb-3.2/drivers/usb/core/driver.c
> @@ -31,6 +31,10 @@
>
> #include "usb.h"
>
> +/* Mutual exclusion for drvwrap->is_registered */
> +DEFINE_MUTEX(usb_driver_reg_mutex);
> +EXPORT_SYMBOL_GPL(usb_driver_reg_mutex);
> +
>
> #ifdef CONFIG_HOTPLUG
>
> @@ -815,6 +819,10 @@ int usb_register_device_driver(struct us
> usbcore_name, retval, new_udriver->name);
> }
>
> + mutex_lock(&usb_driver_reg_mutex);
> + new_udriver->drvwrap.is_registered = 1;
> + mutex_unlock(&usb_driver_reg_mutex);
> +
> return retval;
> }
> EXPORT_SYMBOL_GPL(usb_register_device_driver);
> @@ -831,6 +839,10 @@ void usb_deregister_device_driver(struct
> pr_info("%s: deregistering device driver %s\n",
> usbcore_name, udriver->name);
>
> + mutex_lock(&usb_driver_reg_mutex);
> + udriver->drvwrap.is_registered = 0;
> + mutex_unlock(&usb_driver_reg_mutex);
> +
> driver_unregister(&udriver->drvwrap.driver);
> usbfs_update_special();
> }
> @@ -873,6 +885,10 @@ int usb_register_driver(struct usb_drive
> if (retval)
> goto out;
>
> + mutex_lock(&usb_driver_reg_mutex);
> + new_driver->drvwrap.is_registered = 1;
> + mutex_unlock(&usb_driver_reg_mutex);
> +
> usbfs_update_special();
>
> retval = usb_create_newid_file(new_driver);
> @@ -917,6 +933,10 @@ void usb_deregister(struct usb_driver *d
> pr_info("%s: deregistering interface driver %s\n",
> usbcore_name, driver->name);
>
> + mutex_lock(&usb_driver_reg_mutex);
> + driver->drvwrap.is_registered = 0;
> + mutex_unlock(&usb_driver_reg_mutex);
> +
> usb_remove_removeid_file(driver);
> usb_remove_newid_file(driver);
> usb_free_dynids(driver);
> Index: usb-3.2/drivers/usb/serial/bus.c
> ===================================================================
> --- usb-3.2.orig/drivers/usb/serial/bus.c
> +++ usb-3.2/drivers/usb/serial/bus.c
> @@ -119,13 +119,20 @@ static int usb_serial_device_remove(stru
> static ssize_t store_new_id(struct device_driver *driver,
> const char *buf, size_t count)
> {
> - struct usb_serial_driver *usb_drv = to_usb_serial_driver(driver);
> - ssize_t retval = usb_store_new_id(&usb_drv->dynids, driver, buf, count);
> -
> - if (retval >= 0 && usb_drv->usb_driver != NULL)
> - retval = usb_store_new_id(&usb_drv->usb_driver->dynids,
> - &usb_drv->usb_driver->drvwrap.driver,
> + struct usb_serial_driver *ser_drv = to_usb_serial_driver(driver);
> + struct usb_driver *usb_drv = ser_drv->usb_driver;
> + ssize_t retval = usb_store_new_id(&ser_drv->dynids, driver, buf, count);
> +
> + if (retval >= 0 && usb_drv != NULL) {
> + mutex_lock(&usb_driver_reg_mutex);
> + if (usb_drv->drvwrap.is_registered)
> + retval = usb_store_new_id(&usb_drv->dynids,
> + &usb_drv->drvwrap.driver,
> buf, count);
> + else
> + retval = -EAGAIN;
> + mutex_unlock(&usb_driver_reg_mutex);
> + }
> return retval;
> }
>
> @@ -178,7 +185,7 @@ int usb_serial_bus_register(struct usb_s
>
> void usb_serial_bus_deregister(struct usb_serial_driver *driver)
> {
> - free_dynids(driver);
> driver_unregister(&driver->driver);
> + free_dynids(driver);
> }
>
> Index: usb-3.2/include/linux/usb.h
> ===================================================================
> --- usb-3.2.orig/include/linux/usb.h
> +++ usb-3.2/include/linux/usb.h
> @@ -792,14 +792,18 @@ extern ssize_t usb_store_new_id(struct u
> struct device_driver *driver,
> const char *buf, size_t count);
>
> +extern struct mutex usb_driver_reg_mutex;
> +
> /**
> * struct usbdrv_wrap - wrapper for driver-model structure
> * @driver: The driver-model core driver structure.
> * @for_devices: Non-zero for device drivers, 0 for interface drivers.
> + * @is_registered: The driver is registered (protected by
> usb_driver_reg_mutex) */
> struct usbdrv_wrap {
> struct device_driver driver;
> - int for_devices;
> + unsigned int for_devices:1;
> + unsigned int is_registered:1;
> };
>
> /**
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
