Messages in this thread | | | Date | Thu, 7 Jun 2012 17:30:00 +0800 | Subject | Re: [PATCH] driver core: fix shutdown races with probe/remove | From | Ming Lei <> |
| |
On Wed, Jun 6, 2012 at 10:44 PM, Alan Stern <stern@rowland.harvard.edu> 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-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/
| |