[PATCH] add help to WAIT_SCAN option

[PATCH] add help to WAIT_SCAN option

[Grant Grundler]

Thibaut Varene's parisc box didn't find the root disk.
Problem is he had ASYNC_SCAN enabled and the required SYM2 driver builtin
but the root disk wasn't discovered before the kernel wanted to mount root.
Oh, and he (like many folks who build their own kernels) didn't use
an initrd or initramfs. So no chance to load scsi_wait_scan module.

This patch enables one to select "y" for WAIT_SCAN option and adds
help text on how/when to use "m" vs "y".

Signed-off-by: Grant Grundler <grundler@parisc-linux.org>


diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index d28c14e..a2eefb2 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -235,17 +235,31 @@ config SCSI_SCAN_ASYNC
 	  be a problem as the devices may not have been found by the
 	  time your system expects them to have been.  You can load the
 	  scsi_wait_scan module to ensure that all scans have completed.
+	  (Requires SCSI_WAIT_SCAN config option be enabled).
 	  If you build your SCSI drivers into the kernel, then everything
-	  will work fine if you say Y here.
+	  will work fine if you say Y here and Y to SCSI_WAIT_SCAN option.
 
 	  You can override this choice by specifying "scsi_mod.scan=sync"
 	  or async on the kernel's command line.
 
 config SCSI_WAIT_SCAN
-	tristate
+	tristate "Wait until all async SCSI bus scans have completed?"
 	default m
 	depends on SCSI
 	depends on MODULES
+	help
+	  This module will not allow the boot to proceed until all
+	  the async SCSI bus scans are complete. This is needed
+	  if the root disk is not discovered before the kernel
+	  init sequence attempts to mount root.
+
+	  To use WITH an initrd/initramfs, say "m" (default) and set up
+	  the init scripts to "modprobe scsi_wait_scan" after all the
+	  SCSI HBA drivers have been loaded.
+
+	  To use WITHOUT an initrd/initramfs, say "y" and make sure
+	  all SCSI HBA drivers needed to auto-mount all file systems
+	  (including the root disk) are also "y".
 
 menu "SCSI Transports"
 	depends on SCSI

Re: [PATCH] add help to WAIT_SCAN option

[James Bottomley]

On Sat, 2007-05-26 at 13:11 -0600, Grant Grundler wrote:
> Thibaut Varene's parisc box didn't find the root disk.
> Problem is he had ASYNC_SCAN enabled and the required SYM2 driver builtin
> but the root disk wasn't discovered before the kernel wanted to mount root.
> Oh, and he (like many folks who build their own kernels) didn't use
> an initrd or initramfs. So no chance to load scsi_wait_scan module.
> 
> This patch enables one to select "y" for WAIT_SCAN option and adds
> help text on how/when to use "m" vs "y".

This solution is still as wrong as it was the last three times it was
submitted to linux-scsi:

http://marc.info/?t=117906989600001
http://marc.info/?t=117926627700012
http://marc.info/?t=117923062000004

A better, but still not quite right fix is here:

http://marc.info/?l=linux-scsi&m=117995785931981

James

Re: [PATCH] add help to WAIT_SCAN option

[Matthew Wilcox]

On Sat, May 26, 2007 at 01:11:27PM -0600, Grant Grundler wrote:
> Thibaut Varene's parisc box didn't find the root disk.
> Problem is he had ASYNC_SCAN enabled and the required SYM2 driver builtin
> but the root disk wasn't discovered before the kernel wanted to mount root.
> Oh, and he (like many folks who build their own kernels) didn't use
> an initrd or initramfs. So no chance to load scsi_wait_scan module.

I don't know what Thibaut's problem was, but this explanation is
definitely incorrect.  If you build in the sym2 driver and the scsi
core, then we wait for the discs to be discovered automatically.

Re: [PATCH] add help to WAIT_SCAN option

[Thibaut VARENE]

On 5/26/07, Matthew Wilcox <matthew@wil.cx> wrote:
> On Sat, May 26, 2007 at 01:11:27PM -0600, Grant Grundler wrote:
> > Thibaut Varene's parisc box didn't find the root disk.
> > Problem is he had ASYNC_SCAN enabled and the required SYM2 driver builtin
> > but the root disk wasn't discovered before the kernel wanted to mount root.
> > Oh, and he (like many folks who build their own kernels) didn't use
> > an initrd or initramfs. So no chance to load scsi_wait_scan module.
>
> I don't know what Thibaut's problem was, but this explanation is
> definitely incorrect.  If you build in the sym2 driver and the scsi
> core, then we wait for the discs to be discovered automatically.

My problem was that:
- I use a standalone kernel (no init{rd,ramfs})
- I have CONFIG_MODULES=y
- I build all my (needed) scsi drivers 'y' (builtin)

Yet CONFIG_SCSI_WAIT_SCAN defaults to 'm' with no possibility of
setting it otherwise. So I ended up with a kernel with builtin async
scsi scan and no wait_scan, thus not waiting for the probes to
complete before trying to mount rootfs...

Editing KConfig to allow setting CONFIG_SCSI_WAIT_SCAN=y "fixed" the problem.

HTH

T-Bone

-- 
Thibaut VARENE
http://www.parisc-linux.org/~varenet/

Re: [PATCH] add help to WAIT_SCAN option

[Matthew Wilcox]

On Sun, May 27, 2007 at 12:25:22AM +0200, Thibaut VARENE wrote:
> My problem was that:
> - I use a standalone kernel (no init{rd,ramfs})
> - I have CONFIG_MODULES=y
> - I build all my (needed) scsi drivers 'y' (builtin)

> Yet CONFIG_SCSI_WAIT_SCAN defaults to 'm' with no possibility of
> setting it otherwise. So I ended up with a kernel with builtin async
> scsi scan and no wait_scan, thus not waiting for the probes to
> complete before trying to mount rootfs...

Thanks.  That's expected to work, as it's how I build my kernels too.
Looks like this got broken when other people were trying to fix the
modular case.  It *was* the case that scsi_wait_scan got built in to the
kernel if CONFIG_SCSI was =y.  But of course, that's no good for people
who build with CONFIG_SCSI=y and their driver =m -- they still have to
have a module to load.  And we shouldn't prompt the user for it, we
should determine the right thing to do.

Not even compile tested (but it's at least approximately what the
original async scanning patches used to do):

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index a67f315..2653a06 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -184,6 +184,11 @@ int scsi_complete_async_scans(void)
 /* Only exported for the benefit of scsi_wait_scan */
 EXPORT_SYMBOL_GPL(scsi_complete_async_scans);
 
+/* If everything's built in, we need to wait for everything to scan */
+#ifndef MODULE
+late_initcall(scsi_complete_async_scans);
+#endif
+
 /**
  * scsi_unlock_floptical - unlock device via a special MODE SENSE command
  * @sdev:	scsi device to send command to

Re: [PATCH] add help to WAIT_SCAN option

[Thibaut VARENE]

On 5/27/07, Matthew Wilcox <matthew@wil.cx> wrote:
> On Sun, May 27, 2007 at 12:25:22AM +0200, Thibaut VARENE wrote:
> > My problem was that:
> > - I use a standalone kernel (no init{rd,ramfs})
> > - I have CONFIG_MODULES=y
> > - I build all my (needed) scsi drivers 'y' (builtin)
>
> > Yet CONFIG_SCSI_WAIT_SCAN defaults to 'm' with no possibility of
> > setting it otherwise. So I ended up with a kernel with builtin async
> > scsi scan and no wait_scan, thus not waiting for the probes to
> > complete before trying to mount rootfs...
>
> Thanks.  That's expected to work, as it's how I build my kernels too.
> Looks like this got broken when other people were trying to fix the
> modular case.  It *was* the case that scsi_wait_scan got built in to the
> kernel if CONFIG_SCSI was =y.  But of course, that's no good for people
> who build with CONFIG_SCSI=y and their driver =m -- they still have to
> have a module to load.  And we shouldn't prompt the user for it, we
> should determine the right thing to do.
>
> Not even compile tested (but it's at least approximately what the
> original async scanning patches used to do):
>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c

Works For Me (tm)

HTH

T-Bone

-- 
Thibaut VARENE
http://www.parisc-linux.org/~varenet/

Re: [PATCH] add help to WAIT_SCAN option

[Matthew Wilcox]

On Sun, May 27, 2007 at 01:10:36AM +0200, Thibaut VARENE wrote:
> >diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> 
> Works For Me (tm)

Great, thanks!

James, can you put this patch into your tree?

-----

When all scsi drivers are built-in, we need to automatically call
scsi_complete_async_scans before we need to find the root filesystem.
This used to be done by compiling scsi_wait_scan into the kernel, but
that doesn't work when some scsi drivers are modular.  Instead, just add
a late_initcall() when the SCSI subsystem is built-in.

Signed-off-by: Matthew Wilcox <matthew@wil.cx>

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index a67f315..2653a06 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -184,6 +184,11 @@ int scsi_complete_async_scans(void)
 /* Only exported for the benefit of scsi_wait_scan */
 EXPORT_SYMBOL_GPL(scsi_complete_async_scans);
 
+/* If everything's built in, we need to wait for everything to scan */
+#ifndef MODULE
+late_initcall(scsi_complete_async_scans);
+#endif
+
 /**
  * scsi_unlock_floptical - unlock device via a special MODE SENSE command
  * @sdev:	scsi device to send command to

Re: [PATCH] add help to WAIT_SCAN option

[Grant Grundler]

On Sat, May 26, 2007 at 02:50:14PM -0500, James Bottomley wrote:
> This solution is still as wrong as it was the last three times it was
> submitted to linux-scsi:
> 
> http://marc.info/?t=117906989600001
> http://marc.info/?t=117926627700012
> http://marc.info/?t=117923062000004

ugh...sorry.  I should have looked first.

> A better, but still not quite right fix is here:
> 
> http://marc.info/?l=linux-scsi&m=117995785931981

Yes, I like this a lot better too.
But I'm failing to understanding why "late_initcall" isn't ok. 
I was expecting all drivers to have registered with scsi subsystem
in their module_init() calls and have the scans get kicked off
at that point. Further, I assumed "late_initcall" gets called
after "init" list is processed. What am I missing?

thanks,
grant

Re: [PATCH] add help to WAIT_SCAN option

[James Bottomley]

On Sat, 2007-05-26 at 18:06 -0600, Grant Grundler wrote:
> On Sat, May 26, 2007 at 02:50:14PM -0500, James Bottomley wrote:
> > This solution is still as wrong as it was the last three times it was
> > submitted to linux-scsi:
> > 
> > http://marc.info/?t=117906989600001
> > http://marc.info/?t=117926627700012
> > http://marc.info/?t=117923062000004
> 
> ugh...sorry.  I should have looked first.
> 
> > A better, but still not quite right fix is here:
> > 
> > http://marc.info/?l=linux-scsi&m=117995785931981
> 
> Yes, I like this a lot better too.
> But I'm failing to understanding why "late_initcall" isn't ok. 
> I was expecting all drivers to have registered with scsi subsystem
> in their module_init() calls and have the scans get kicked off
> at that point. Further, I assumed "late_initcall" gets called
> after "init" list is processed. What am I missing?

Actually, it's a thinko on my part ... I thought late_initcall() and
module_init() are the same level, and hence this would be link order
dependent.  Actually that's only true in the modular case, so the patch
is fine ... I'll put it in.

James

Re: [PATCH] add help to WAIT_SCAN option

[Matthew Wilcox]

On Sat, May 26, 2007 at 07:15:43PM -0500, James Bottomley wrote:
> Actually, it's a thinko on my part ... I thought late_initcall() and
> module_init() are the same level, and hence this would be link order
> dependent.  Actually that's only true in the modular case, so the patch
> is fine ... I'll put it in.

I disagree that Grant's patch is fine.  Consider:

 - CONFIG_SCSI=y
 - CONFIG_SCSI_SYM2=y
 - CONFIG_SCSI_QLA2XYZ=m

You want the root disc on your sym2 to be found, so you need
SCSI_WAIT_SCAN=y, but you also want to know when qla2xyz has finished
scanning the fabric, so you need SCSI_WAIT_SCAN=m.  My patch doesn't
have this problem.

Re: [PATCH] add help to WAIT_SCAN option

[James Bottomley]

On Sat, 2007-05-26 at 18:27 -0600, Matthew Wilcox wrote:
> On Sat, May 26, 2007 at 07:15:43PM -0500, James Bottomley wrote:
> > Actually, it's a thinko on my part ... I thought late_initcall() and
> > module_init() are the same level, and hence this would be link order
> > dependent.  Actually that's only true in the modular case, so the patch
> > is fine ... I'll put it in.
> 
> I disagree that Grant's patch is fine.  Consider:
> 
>  - CONFIG_SCSI=y
>  - CONFIG_SCSI_SYM2=y
>  - CONFIG_SCSI_QLA2XYZ=m
> 
> You want the root disc on your sym2 to be found, so you need
> SCSI_WAIT_SCAN=y, but you also want to know when qla2xyz has finished
> scanning the fabric, so you need SCSI_WAIT_SCAN=m.  My patch doesn't
> have this problem.

Er .. I think you misunderstood the email.  Grant asked why Hugh's (not
Grant's) patch was wrong and I said that on reflection it wasn't.

James

Re: [PATCH] add help to WAIT_SCAN option

[Matthew Wilcox]

On Sat, May 26, 2007 at 07:29:30PM -0500, James Bottomley wrote:
> Er .. I think you misunderstood the email.  Grant asked why Hugh's (not
> Grant's) patch was wrong and I said that on reflection it wasn't.

Oh, right.  Since Hugh's patch is the same as the one I sent in the
other fork of this thread, except for the contents of the comment, by
all means put in Hugh's version instead of mine.  Include my signed-off
if you like.

Re: [PATCH] add help to WAIT_SCAN option

[Grant Grundler]

On Sat, May 26, 2007 at 06:33:10PM -0600, Matthew Wilcox wrote:
> On Sat, May 26, 2007 at 07:29:30PM -0500, James Bottomley wrote:
> > Er .. I think you misunderstood the email.  Grant asked why Hugh's (not
> > Grant's) patch was wrong and I said that on reflection it wasn't.
> 
> Oh, right.  Since Hugh's patch is the same as the one I sent in the
> other fork of this thread, except for the contents of the comment, by
> all means put in Hugh's version instead of mine.  Include my signed-off
> if you like.

And you can add an "Acked-by: Grant Grundler <grundler@parisc-linux.org>"
since I prodded you to reconsider the patch. :)

thanks,
grant