On Wed, Jun 6, 2012 at 10:44 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: >> > On the whole, it might be easier just to hold the device lock during >> > the shutdown call. >> >> Yes, it is easier, but it is not efficient because there are very less >> drivers which implemented .shutdown callback. Suppose there are some >> drivers which have no .shutdown but are holding device lock, extra >> time will be consumed in the lock acquiring, which may slow down 'power >> down'. > > The main reasons for holding the device lock are probe and remove > callbacks. In your patch, synchronize_srcu has to wait for those too. > So there won't be much extra time. There are differences between holding dev->lock(parent->lock) and using synchronize_srcu: - synchronize_srcu just wait for the ongoing probe/release to complete - holding dev->lock and parent->lock before shutdown means every dev->lock and parent->lock is to be acquired, so any drivers holding dev lock during device_shutdown will slow down 'power down' or 'reset'. - if the device has not .shutdown implemented, it is not necessary to acquire the lock and parent->lock, either of which may has been held in other places already. But after thinking about the problem further, the patch isn't good enough: - even probe/remove is disallowed during shutdown, device_add/ device_del can be completed and only probe and release things are bypassed, so .shutdown may see a deleted device from view of driver model. For example, pci_device_shutdown need to access its devres which may have been deleted by device_del already. - violate driver model's implicit rule: once device_release_driver (in device_del path) is completed, other callbacks for the device(driver) won't be seen or called by others So take the simply approach of holding lock to fix the races. Thanks, -- Ming Lei -- 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
