On Tue, 2018-11-06 at 08:18 -0800, Alexander Duyck wrote:
On Mon, 2018-11-05 at 17:04 -0800, Bart Van Assche wrote:
quoted
On Mon, 2018-11-05 at 13:11 -0800, Alexander Duyck wrote:
quoted
This patch moves the async_synchronize_full call out of
__device_release_driver and into driver_detach.
The idea behind this is that the async_synchronize_full call will only
guarantee that any existing async operations are flushed. This doesn't do
anything to guarantee that a hotplug event that may occur while we are
doing the release of the driver will not be asynchronously scheduled.
By moving this into the driver_detach path we can avoid potential deadlocks
as we aren't holding the device lock at this point and we should not have
the driver we want to flush loaded so the flush will take care of any
asynchronous events the driver we are detaching might have scheduled.
Signed-off-by: Alexander Duyck <redacted>
---
drivers/base/dd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 76c40fe69463..e74cefeb5b69 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -975,9 +975,6 @@ static void __device_release_driver(struct device *dev, struct device *parent)
drv = dev->driver;
if (drv) {
- if (driver_allows_async_probing(drv))
- async_synchronize_full();
-
while (device_links_busy(dev)) {
__device_driver_unlock(dev, parent);
@@ -1087,6 +1084,9 @@ void driver_detach(struct device_driver *drv)
struct device_private *dev_prv;
struct device *dev;
+ if (driver_allows_async_probing(drv))
+ async_synchronize_full();
+
for (;;) {
spin_lock(&drv->p->klist_devices.k_lock);
if (list_empty(&drv->p->klist_devices.k_list)) {
Have you considered to move that async_synchronize_full() call into
bus_remove_driver()? Verifying the correctness of this patch requires to
check whether the async_synchronize_full() comes after the
klist_remove(&drv->p->knode_bus) call. That verification is easier when
the async_synchronize_full() call occurs in bus_remove_driver() instead
of in driver_detach().
I considered it, however it ends up with things being more symmetric to
have use take care of synchronizing things in driver_detach since after
this patch set we are scheduling thing asynchronously in driver_attach.
Also I don't think it would be any great risk simply because calling
driver_detach with the driver still associated with the bus would be a
blatent error as it could easily lead to issues where you unbind a
driver but have it get hotplugged to a device while that is going on.
Thanks for the additional clarification. Since I'm fine with this patch:
Reviewed-by: Bart Van Assche <redacted>