Re: [driver-core PATCH v5 4/9] driver core: Move async_synchronize_full call

[Bart Van Assche]

On Mon, 2018-11-05 at 13:11 -0800, Alexander Duyck wrote:
+AD4 This patch moves the async+AF8-synchronize+AF8-full call out of
+AD4 +AF8AXw-device+AF8-release+AF8-driver and into driver+AF8-detach.
+AD4 
+AD4 The idea behind this is that the async+AF8-synchronize+AF8-full call will only
+AD4 guarantee that any existing async operations are flushed. This doesn't do
+AD4 anything to guarantee that a hotplug event that may occur while we are
+AD4 doing the release of the driver will not be asynchronously scheduled.
+AD4 
+AD4 By moving this into the driver+AF8-detach path we can avoid potential deadlocks
+AD4 as we aren't holding the device lock at this point and we should not have
+AD4 the driver we want to flush loaded so the flush will take care of any
+AD4 asynchronous events the driver we are detaching might have scheduled.
+AD4 
+AD4 Signed-off-by: Alexander Duyck +ADw-alexander.h.duyck+AEA-linux.intel.com+AD4
+AD4 ---
+AD4  drivers/base/dd.c +AHw    6 +-+-+----
+AD4  1 file changed, 3 insertions(+-), 3 deletions(-)
+AD4 
+AD4 diff --git a/drivers/base/dd.c b/drivers/base/dd.c
+AD4 index 76c40fe69463..e74cefeb5b69 100644
+AD4 --- a/drivers/base/dd.c
+AD4 +-+-+- b/drivers/base/dd.c
+AD4 +AEAAQA -975,9 +-975,6 +AEAAQA static void +AF8AXw-device+AF8-release+AF8-driver(struct device +ACo-dev, struct device +ACo-parent)
+AD4  
+AD4  	drv +AD0 dev-+AD4-driver+ADs
+AD4  	if (drv) +AHs
+AD4 -		if (driver+AF8-allows+AF8-async+AF8-probing(drv))
+AD4 -			async+AF8-synchronize+AF8-full()+ADs
+AD4 -
+AD4  		while (device+AF8-links+AF8-busy(dev)) +AHs
+AD4  			+AF8AXw-device+AF8-driver+AF8-unlock(dev, parent)+ADs
+AD4  
+AD4 +AEAAQA -1087,6 +-1084,9 +AEAAQA void driver+AF8-detach(struct device+AF8-driver +ACo-drv)
+AD4  	struct device+AF8-private +ACo-dev+AF8-prv+ADs
+AD4  	struct device +ACo-dev+ADs
+AD4  
+AD4 +-	if (driver+AF8-allows+AF8-async+AF8-probing(drv))
+AD4 +-		async+AF8-synchronize+AF8-full()+ADs
+AD4 +-
+AD4  	for (+ADsAOw) +AHs
+AD4  		spin+AF8-lock(+ACY-drv-+AD4-p-+AD4-klist+AF8-devices.k+AF8-lock)+ADs
+AD4  		if (list+AF8-empty(+ACY-drv-+AD4-p-+AD4-klist+AF8-devices.k+AF8-list)) +AHs

Have you considered to move that async+AF8-synchronize+AF8-full() call into
bus+AF8-remove+AF8-driver()? Verifying the correctness of this patch requires to
check whether the async+AF8-synchronize+AF8-full() comes after the
klist+AF8-remove(+ACY-drv-+AD4-p-+AD4-knode+AF8-bus) call. That verification is easier when
the async+AF8-synchronize+AF8-full() call occurs in bus+AF8-remove+AF8-driver() instead
of in driver+AF8-detach().

Thanks,

Bart.