2009-04-21 20:40:26

by Arjan van de Ven

[permalink] [raw]
Subject: driver synchronization: make scsi_wait_scan more advanced

(CC'ing linux-scsi as well since this touches a file in drivers/scsi)


>From 546fdda8b4124a7473f32decf4a2981984f287db Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <[email protected]>
Date: Tue, 21 Apr 2009 13:32:54 -0700
Subject: [PATCH] driver synchronization: make scsi_wait_scan more advanced


There is currently only one way for userspace to say "wait for my storage
device to get ready for the modules I just loaded": to load the
scsi_wait_scan module. Expectations of userspace are that once this
module is loaded, all the (storage) devices for which the drivers
were loaded before the module load are present.

Now, there are some issues with the implementation, and the async
stuff got caught in the middle of this: The existing code only
waits for the scsy async probing to finish, but it did not take
into account at all that probing might not have begun yet.
(Russell ran into this problem on his computer and the fix works for him)

This patch fixes this more thoroughly than the previous "fix", which
had some bad side effects (namely, for kernel code that wanted to wait for
the scsi scan it would also do an async sync, which would deadlock if you did
it from async context already.. there's a report about that on lkml):
The patch makes the module first wait for all device driver probes, and then it
will wait for the scsi parallel scan to finish.

Signed-off-by: Arjan van de Ven <[email protected]>
---
drivers/base/dd.c | 1 +
drivers/scsi/scsi_scan.c | 2 --
drivers/scsi/scsi_wait_scan.c | 11 +++++++++++
include/linux/device.h | 1 +
4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index f17c326..742cbe6 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -179,6 +179,7 @@ void wait_for_device_probe(void)
wait_event(probe_waitqueue, atomic_read(&probe_count) == 0);
async_synchronize_full();
}
+EXPORT_SYMBOL_GPL(wait_for_device_probe);

/**
* driver_probe_device - attempt to bind device & driver together
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index a14d245..6f51ca4 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -180,8 +180,6 @@ int scsi_complete_async_scans(void)
spin_unlock(&async_scan_lock);

kfree(data);
- /* Synchronize async operations globally */
- async_synchronize_full();
return 0;
}

diff --git a/drivers/scsi/scsi_wait_scan.c b/drivers/scsi/scsi_wait_scan.c
index 2f21af2..74708fc 100644
--- a/drivers/scsi/scsi_wait_scan.c
+++ b/drivers/scsi/scsi_wait_scan.c
@@ -11,10 +11,21 @@
*/

#include <linux/module.h>
+#include <linux/device.h>
#include <scsi/scsi_scan.h>

static int __init wait_scan_init(void)
{
+ /*
+ * First we need to wait for device probing to finish;
+ * the drivers we just loaded might just still be probing
+ * and might not yet have reached the scsi async scanning
+ */
+ wait_for_device_probe();
+ /*
+ * and then we wait for the actual asynchronous scsi scan
+ * to finish.
+ */
scsi_complete_async_scans();
return 0;
}
diff --git a/include/linux/device.h b/include/linux/device.h
index 2918c0e..6a69caa 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -551,6 +551,7 @@ extern int (*platform_notify_remove)(struct device *dev);
extern struct device *get_device(struct device *dev);
extern void put_device(struct device *dev);

+extern void wait_for_device_probe(void);

/* drivers/base/power/shutdown.c */
extern void device_shutdown(void);
--
1.6.0.6



--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org


2009-04-21 21:20:06

by Russell King

[permalink] [raw]
Subject: Re: driver synchronization: make scsi_wait_scan more advanced

On Tue, Apr 21, 2009 at 01:42:06PM -0700, Arjan van de Ven wrote:
> (CC'ing linux-scsi as well since this touches a file in drivers/scsi)
>
>
> From 546fdda8b4124a7473f32decf4a2981984f287db Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <[email protected]>
> Date: Tue, 21 Apr 2009 13:32:54 -0700
> Subject: [PATCH] driver synchronization: make scsi_wait_scan more advanced
>
>
> There is currently only one way for userspace to say "wait for my storage
> device to get ready for the modules I just loaded": to load the
> scsi_wait_scan module. Expectations of userspace are that once this
> module is loaded, all the (storage) devices for which the drivers
> were loaded before the module load are present.
>
> Now, there are some issues with the implementation, and the async
> stuff got caught in the middle of this: The existing code only
> waits for the scsy async probing to finish, but it did not take
> into account at all that probing might not have begun yet.
> (Russell ran into this problem on his computer and the fix works for him)

Indeed yes. With CONFIG_SCSI_SCAN_ASYNC unset, but with the scsi host
driver built as a module, the behaviour without this patch causes
problems. The machine would load the pata_pcmcia driver, which would
then detect and register the interface:

ata1: PATA max PIO0 cmd 0xd0880000 ctl 0xd088000e irq 135

At this point, the insmod process returns, and the init script continued,
loading scsi_wait_scan which did nothing, and we'd continue the init
script, which would try to detect and mount the storage attached to this
interface - and fail because the device probes haven't completed. The
only workaround I found was to add a 'sleep 1' to the script to give the
various layers in the kernel time to sort themselves out.

With this patch, the loading of scsi_wait_scan waits for the device to
finish probing, and we can properly detect and mount the attached storage
(which in this case is the rootfs) and the 'sleep 1' hack isn't required.

So, for everything but the scsi_scan.c hunk:

Tested-by: Russell King <[email protected]>

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: