2007-05-13 15:23:35

by Robert P. J. Day

[permalink] [raw]
Subject: why does x86 "make defconfig" build a single, lonely module?


not a big deal, but is there a reason that a "make defconfig" on my
x86 system ends up selecting and building a single module?

Building modules, stage 2.
MODPOST 1 modules
CC drivers/scsi/scsi_wait_scan.mod.o
LD [M] drivers/scsi/scsi_wait_scan.ko

is there something special about that module? just curious.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================


2007-05-13 15:59:17

by Jan Engelhardt

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?


On May 13 2007 11:22, Robert P. J. Day wrote:
>
> not a big deal, but is there a reason that a "make defconfig" on my
>x86 system ends up selecting and building a single module?
>
> Building modules, stage 2.
> MODPOST 1 modules
> CC drivers/scsi/scsi_wait_scan.mod.o
> LD [M] drivers/scsi/scsi_wait_scan.ko
>
>is there something special about that module? just curious.

$ make defconfig;
$ grep =m .config;
CONFIG_SCSI_WAIT_SCAN=m
$ grep -A2 SCSI_WAIT_SCAN drivers/scsi/Kconfig;
config SCSI_WAIT_SCAN
tristate
default m
$ grep SCSI_WAIT_SCAN arch/i386/Kconfig || echo "no result";
no result



Jan
--

2007-05-13 16:06:26

by Dave Jones

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, May 13, 2007 at 11:22:55AM -0400, Robert P. J. Day wrote:
>
> not a big deal, but is there a reason that a "make defconfig" on my
> x86 system ends up selecting and building a single module?
>
> Building modules, stage 2.
> MODPOST 1 modules
> CC drivers/scsi/scsi_wait_scan.mod.o
> LD [M] drivers/scsi/scsi_wait_scan.ko
>
> is there something special about that module? just curious.

My guess is that someone was paranoid and wanted not to have
to answer a zillion "my machine won't boot any more" questions
when async scsi scanning was added.
This might further clarify..

---

The scsi_wait_scan module is only really useful if async scanning
is enabled.

Signed-off-by: Dave Jones <[email protected]>

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e62d23f..0f6c370 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -244,7 +244,7 @@ config SCSI_SCAN_ASYNC
config SCSI_WAIT_SCAN
tristate
default m
- depends on SCSI
+ depends on SCSI_SCAN_ASYNC
depends on MODULES

menu "SCSI Transports"



--
http://www.codemonkey.org.uk

2007-05-13 16:11:20

by James Bottomley

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, 2007-05-13 at 12:06 -0400, Dave Jones wrote:
> On Sun, May 13, 2007 at 11:22:55AM -0400, Robert P. J. Day wrote:
> >
> > not a big deal, but is there a reason that a "make defconfig" on my
> > x86 system ends up selecting and building a single module?
> >
> > Building modules, stage 2.
> > MODPOST 1 modules
> > CC drivers/scsi/scsi_wait_scan.mod.o
> > LD [M] drivers/scsi/scsi_wait_scan.ko
> >
> > is there something special about that module? just curious.
>
> My guess is that someone was paranoid and wanted not to have
> to answer a zillion "my machine won't boot any more" questions
> when async scsi scanning was added.
> This might further clarify..
>
> ---
>
> The scsi_wait_scan module is only really useful if async scanning
> is enabled.
>
> Signed-off-by: Dave Jones <[email protected]>
>
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index e62d23f..0f6c370 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -244,7 +244,7 @@ config SCSI_SCAN_ASYNC
> config SCSI_WAIT_SCAN
> tristate
> default m
> - depends on SCSI
> + depends on SCSI_SCAN_ASYNC

No. SCSI_SCAN_ASYNC is a bool ... if you depend on it, you'll force the
wait scan to be built in, which isn't the idea at all.

James


2007-05-13 16:18:54

by James Bottomley

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, 2007-05-13 at 11:10 -0500, James Bottomley wrote:
> > - depends on SCSI
> > + depends on SCSI_SCAN_ASYNC
>
> No. SCSI_SCAN_ASYNC is a bool ... if you depend on it, you'll force the
> wait scan to be built in, which isn't the idea at all.

Plus SCSI_SCAN_ASYNC only sets the *default* for async scanning. You
can alter this at boot time, so you could need the wait scan module even
with it set to N.

James


2007-05-13 16:21:35

by Robert P. J. Day

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, 13 May 2007, James Bottomley wrote:

> On Sun, 2007-05-13 at 12:06 -0400, Dave Jones wrote:
> > On Sun, May 13, 2007 at 11:22:55AM -0400, Robert P. J. Day wrote:
> > >
> > > not a big deal, but is there a reason that a "make defconfig" on my
> > > x86 system ends up selecting and building a single module?
> > >
> > > Building modules, stage 2.
> > > MODPOST 1 modules
> > > CC drivers/scsi/scsi_wait_scan.mod.o
> > > LD [M] drivers/scsi/scsi_wait_scan.ko
> > >
> > > is there something special about that module? just curious.
> >
> > My guess is that someone was paranoid and wanted not to have
> > to answer a zillion "my machine won't boot any more" questions
> > when async scsi scanning was added.
> > This might further clarify..
> >
> > ---
> >
> > The scsi_wait_scan module is only really useful if async scanning
> > is enabled.
> >
> > Signed-off-by: Dave Jones <[email protected]>
> >
> > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> > index e62d23f..0f6c370 100644
> > --- a/drivers/scsi/Kconfig
> > +++ b/drivers/scsi/Kconfig
> > @@ -244,7 +244,7 @@ config SCSI_SCAN_ASYNC
> > config SCSI_WAIT_SCAN
> > tristate
> > default m
> > - depends on SCSI
> > + depends on SCSI_SCAN_ASYNC
>
> No. SCSI_SCAN_ASYNC is a bool ... if you depend on it, you'll force
> the wait scan to be built in, which isn't the idea at all.

since this thread looks like it's going to get away from me in a
hurry :-), my only point in asking was to point out that that lone
module was the only thing preventing the build from being module-free.

i'm not saying that that's *necessarily* a good thing, but it just
strikes me as odd that, out of all of the possible modules that might
be selected in a default config for x86, this was the *only* one that
was picked.

i just think it's a bit weird, that's all.

rday

p.s. it's mostly a case of -- whenever i notice something being done
only *once* in the entire source tree, i'm always a bit leery.

--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

2007-05-13 16:28:16

by James Bottomley

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, 2007-05-13 at 12:20 -0400, Robert P. J. Day wrote:
> since this thread looks like it's going to get away from me in a
> hurry :-), my only point in asking was to point out that that lone
> module was the only thing preventing the build from being module-free.
>
> i'm not saying that that's *necessarily* a good thing, but it just
> strikes me as odd that, out of all of the possible modules that might
> be selected in a default config for x86, this was the *only* one that
> was picked.
>
> i just think it's a bit weird, that's all.

It's designed on the predicate that people who want to be module free
will actually set CONFIG_MODULE=n.

If you set CONFIG_MODULE=y and build SCSI we assume you could have a
SCSI driver module at some point, which would necessitate the wait scan
module.

James


2007-05-13 16:29:00

by Dave Jones

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, May 13, 2007 at 11:10:55AM -0500, James Bottomley wrote:

> > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> > index e62d23f..0f6c370 100644
> > --- a/drivers/scsi/Kconfig
> > +++ b/drivers/scsi/Kconfig
> > @@ -244,7 +244,7 @@ config SCSI_SCAN_ASYNC
> > config SCSI_WAIT_SCAN
> > tristate
> > default m
> > - depends on SCSI
> > + depends on SCSI_SCAN_ASYNC
>
> No. SCSI_SCAN_ASYNC is a bool ... if you depend on it, you'll force the
> wait scan to be built in, which isn't the idea at all.

hmm?

with my change...

$ make defconfig
$ grep SCSI_SCAN_ASYNC .config
# CONFIG_SCSI_SCAN_ASYNC is not set
$ grep SCSI_WAIT_SCAN .config

<edit .config to remove the CONFIG_SCSI_SCAN_ASYNC line>
$ make oldconfig
<answer 'y' for CONFIG_SCSI_SCAN_ASYNC>

$ grep SCSI_SCAN_ASYNC .config
CONFIG_SCSI_SCAN_ASYNC=y
$ grep SCSI_WAIT_SCAN .config
CONFIG_SCSI_WAIT_SCAN=m


Dave

--
http://www.codemonkey.org.uk

2007-05-13 16:30:39

by Dave Jones

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, May 13, 2007 at 11:18:35AM -0500, James Bottomley wrote:
> On Sun, 2007-05-13 at 11:10 -0500, James Bottomley wrote:
> > > - depends on SCSI
> > > + depends on SCSI_SCAN_ASYNC
> >
> > No. SCSI_SCAN_ASYNC is a bool ... if you depend on it, you'll force the
> > wait scan to be built in, which isn't the idea at all.
>
> Plus SCSI_SCAN_ASYNC only sets the *default* for async scanning. You
> can alter this at boot time, so you could need the wait scan module even
> with it set to N.

Ah, good point.

Dave

--
http://www.codemonkey.org.uk

2007-05-13 16:38:26

by Robert P. J. Day

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, 13 May 2007, James Bottomley wrote:

> On Sun, 2007-05-13 at 12:20 -0400, Robert P. J. Day wrote:
> > since this thread looks like it's going to get away from me in a
> > hurry :-), my only point in asking was to point out that that lone
> > module was the only thing preventing the build from being module-free.
> >
> > i'm not saying that that's *necessarily* a good thing, but it just
> > strikes me as odd that, out of all of the possible modules that might
> > be selected in a default config for x86, this was the *only* one that
> > was picked.
> >
> > i just think it's a bit weird, that's all.
>
> It's designed on the predicate that people who want to be module free
> will actually set CONFIG_MODULE=n.
>
> If you set CONFIG_MODULE=y and build SCSI we assume you could have a
> SCSI driver module at some point, which would necessitate the wait scan
> module.

ok, fair enough. thanks.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

2007-05-13 17:43:11

by Simon Arlott

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On 13/05/07 17:27, James Bottomley wrote:
> On Sun, 2007-05-13 at 12:20 -0400, Robert P. J. Day wrote:
>> since this thread looks like it's going to get away from me in a
>> hurry :-), my only point in asking was to point out that that lone
>> module was the only thing preventing the build from being module-free.
>>
>> i'm not saying that that's *necessarily* a good thing, but it just
>> strikes me as odd that, out of all of the possible modules that might
>> be selected in a default config for x86, this was the *only* one that
>> was picked.
>>
>> i just think it's a bit weird, that's all.
>
> It's designed on the predicate that people who want to be module free
> will actually set CONFIG_MODULE=n.
>
> If you set CONFIG_MODULE=y and build SCSI we assume you could have a
> SCSI driver module at some point, which would necessitate the wait scan
> module.

This should be implemented like "Library routines" and only added if such
a SCSI driver module is actually selected. Why can't it at least be a
visible option in the menu? (Although even then it looks like it's
impossible to disable).

Why does ATA select SCSI anyway? Surely PATA doesn't require it?

--
Simon Arlott

2007-05-13 17:49:24

by James Bottomley

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, 2007-05-13 at 18:42 +0100, Simon Arlott wrote:
> > If you set CONFIG_MODULE=y and build SCSI we assume you could have a
> > SCSI driver module at some point, which would necessitate the wait scan
> > module.
>
> This should be implemented like "Library routines" and only added if such
> a SCSI driver module is actually selected. Why can't it at least be a
> visible option in the menu? (Although even then it looks like it's
> impossible to disable).

There's out of tree modules to consider.

> Why does ATA select SCSI anyway? Surely PATA doesn't require it?

That's a bit offtopic and to the wrong list.

libata-pata does require SCSI ... and I suspect the libata developers
didn't want to trip users by them having to select SCSI before they
could select libata.

James


2007-05-13 18:26:47

by Simon Arlott

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On 13/05/07 18:48, James Bottomley wrote:
> On Sun, 2007-05-13 at 18:42 +0100, Simon Arlott wrote:
>>> If you set CONFIG_MODULE=y and build SCSI we assume you could have a
>>> SCSI driver module at some point, which would necessitate the wait scan
>>> module.
>> This should be implemented like "Library routines" and only added if such
>> a SCSI driver module is actually selected. Why can't it at least be a
>> visible option in the menu? (Although even then it looks like it's
>> impossible to disable).
>
> There's out of tree modules to consider.

Ok, I propose we make dozens of modules default 'm' in case an out of
tree module requires it.

SCSI_SCAN_ASYNC ("Asynchronous SCSI scanning"): "You can load the
scsi_wait_scan module to ensure that all scans have completed."

It looks like SCSI_WAIT_SCAN is pointless unless SCSI_SCAN_ASYNC
is selected - so it should depend on it:

---
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e62d23f..0f6c370 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -244,7 +244,7 @@ config SCSI_SCAN_ASYNC
config SCSI_WAIT_SCAN
tristate
default m
- depends on SCSI
+ depends on SCSI_SCAN_ASYNC
depends on MODULES

menu "SCSI Transports"


--
Simon Arlott

2007-05-13 18:45:28

by Dave Jones

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, May 13, 2007 at 07:26:31PM +0100, Simon Arlott wrote:

> It looks like SCSI_WAIT_SCAN is pointless unless SCSI_SCAN_ASYNC
> is selected - so it should depend on it:
>
> ---
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index e62d23f..0f6c370 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -244,7 +244,7 @@ config SCSI_SCAN_ASYNC
> config SCSI_WAIT_SCAN
> tristate
> default m
> - depends on SCSI
> + depends on SCSI_SCAN_ASYNC
> depends on MODULES
>
> menu "SCSI Transports"

This is the same broken patch that I posted earlier in this thread.
See James' follow-ups to my mail.

Dave

--
http://www.codemonkey.org.uk

2007-05-13 18:45:53

by James Bottomley

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Sun, 2007-05-13 at 19:26 +0100, Simon Arlott wrote:
> On 13/05/07 18:48, James Bottomley wrote:
> > On Sun, 2007-05-13 at 18:42 +0100, Simon Arlott wrote:
> >>> If you set CONFIG_MODULE=y and build SCSI we assume you could have a
> >>> SCSI driver module at some point, which would necessitate the wait scan
> >>> module.
> >> This should be implemented like "Library routines" and only added if such
> >> a SCSI driver module is actually selected. Why can't it at least be a
> >> visible option in the menu? (Although even then it looks like it's
> >> impossible to disable).
> >
> > There's out of tree modules to consider.
>
> Ok, I propose we make dozens of modules default 'm' in case an out of
> tree module requires it.
>
> SCSI_SCAN_ASYNC ("Asynchronous SCSI scanning"): "You can load the
> scsi_wait_scan module to ensure that all scans have completed."
>
> It looks like SCSI_WAIT_SCAN is pointless unless SCSI_SCAN_ASYNC
> is selected - so it should depend on it:

Why don't you just actually read the thread? Then you'd understand why
this isn't correct.

James


2007-05-13 20:39:19

by Simon Arlott

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On 13/05/07 17:10, James Bottomley wrote:
> On Sun, 2007-05-13 at 12:06 -0400, Dave Jones wrote:
>> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
>> index e62d23f..0f6c370 100644
>> --- a/drivers/scsi/Kconfig
>> +++ b/drivers/scsi/Kconfig
>> @@ -244,7 +244,7 @@ config SCSI_SCAN_ASYNC
>> config SCSI_WAIT_SCAN
>> tristate
>> default m
>> - depends on SCSI
>> + depends on SCSI_SCAN_ASYNC
>
> No. SCSI_SCAN_ASYNC is a bool ... if you depend on it, you'll force the
> wait scan to be built in, which isn't the idea at all.

Try it. It doesn't force it to be built in.

> Plus SCSI_SCAN_ASYNC only sets the *default* for async scanning. You
> can alter this at boot time, so you could need the wait scan module even
> with it set to N.

static int __init wait_scan_init(void)
{
scsi_complete_async_scans();
return 0;
}

Could that not be built-in to SCSI as a sysfs attribute, rather than using
a module to tell the kernel to do something? It looks like someone might
want to call scsi_complete_async_scans() more than once too - if they also
don't allow modules to be unloaded then they can't.

--
Simon Arlott

2007-05-14 09:35:49

by Satyam Sharma

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

Hi,

On 5/13/07, James Bottomley <[email protected]> wrote:
> On Sun, 2007-05-13 at 11:10 -0500, James Bottomley wrote:
> > > - depends on SCSI
> > > + depends on SCSI_SCAN_ASYNC

This is incorrect, alright, but not because of any of the reasons James
mentions below.

The only reason why some module (or Kconfig option) should _depend_
on some other Kconfig option is if (*and only if*) it re-uses _code_
exported by said dependency.

In this particular case, SCSI_SCAN_ASYNC=y/n only controls the
default value of some variable somewhere in SCSI, so no other code
can meaningfully "depend" on it.

> > No. SCSI_SCAN_ASYNC is a bool ... if you depend on it, you'll force the
> > wait scan to be built in, which isn't the idea at all.

Umm, this isn't true, actually.

> Plus SCSI_SCAN_ASYNC only sets the *default* for async scanning. You
> can alter this at boot time, so you could need the wait scan module even
> with it set to N.

I think the _correct_ way to handle this situation (and which would make
everyone happy here; Robert can get his module-free builds with defconfig,
James gets his SCSI_WAIT_SCAN module even if nobody asked for it
explicitly) would be as follows:

---

Clean up Kconfig entry for CONFIG_SCSI_WAIT_SCAN.

Signed-off-by: Satyam Sharma <[email protected]>

---

drivers/scsi/Kconfig | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

---

diff -ruNp a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
--- a/drivers/scsi/Kconfig 2007-05-10 23:19:32.000000000 +0530
+++ b/drivers/scsi/Kconfig 2007-05-14 15:12:46.000000000 +0530
@@ -243,9 +243,8 @@ config SCSI_SCAN_ASYNC

config SCSI_WAIT_SCAN
tristate
- default m
- depends on SCSI
- depends on MODULES
+ default m if SCSI=m
+ default n

menu "SCSI Transports"
depends on SCSI

2007-05-14 09:45:20

by Satyam Sharma

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On 5/14/07, Satyam Sharma <[email protected]> wrote:
> [...]
> config SCSI_WAIT_SCAN
> tristate
> - default m
> - depends on SCSI
> - depends on MODULES
> + default m if SCSI=m
> + default n

Note that this also means SCSI_WAIT_SCAN=n (will not get compiled
and built even as a module) if SCSI=y. But this is perfectly fine, because
I see:

#ifndef MODULE
late_initcall(scsi_complete_async_scans);
#endif

in drivers/scsi/scsi_scan.c anyway, so the async scans will be waited
upon and get complete even when SCSI=y and scsi_scan_type=async,
i.e. you don't really need the scsi_wait_scan module in that case anyway.

Satyam

2007-05-14 12:01:29

by Satyam Sharma

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

Hello,

> > [...]
> > config SCSI_WAIT_SCAN
> > tristate
> > - default m
> > - depends on SCSI
> > - depends on MODULES
> > + default m if SCSI=m
> > + default n
>
> Note that this also means SCSI_WAIT_SCAN=n (will not get compiled
> and built even as a module) if SCSI=y. But this is perfectly fine, because
> I see:
>
> #ifndef MODULE
> late_initcall(scsi_complete_async_scans);
> #endif
>
> in drivers/scsi/scsi_scan.c anyway, so the async scans will be waited
> upon and get complete even when SCSI=y and scsi_scan_type=async,
> i.e. you don't really need the scsi_wait_scan module in that case anyway.

It seems there is a:

late_initcall(wait_scan_init);

in drivers/scsi/scsi_wait_scan.c too. So we can get rid of the redundant:

#ifndef MODULE
late_initcal(scsi_complete_async_scans);
#endif

in drivers/scsi/scsi_scan.c by enforcing SCSI_WAIT_SCAN=y when
SCSI=y (and =m when SCSI=m).

I guess this is probably the behaviour that James wanted originally?

Anyway, attached patch (subsumes previous one) cleans up all this.
Now, scsi_wait_scan is the only guy who controls waiting upon async
scans to complete:

If SCSI=n, SCSI_WAIT_SCAN=n, obviously.

If SCSI=y, SCSI_WAIT_SCAN=y, so it gets built-in and is run at
late_initcall.

If SCSI=m, SCSI_WAIT_SCAN=m too and would hopefully be run
by the initrd/initramfs scripts to wait for async SCSI bus scans to
finish before allowing the boot to proceed.

[ Attached patch does _not_ expose SCSI_WAIT_SCAN to the
user at kernel configuration time, so if somebody wants it to be
built as a module even when SCSI=y (why would anybody
want that, doesn't make much sense to me to modprobe
scsi_wait_scan _after_ boot-up), it would not be possible to
do so. But if someone really wants that, let me know, we can
add a depends, tristate "..." and help in this Kconfig option to
accomplish that too. ]

Thanks,
Satyam

Signed-off-by: Satyam Sharma <[email protected]>

---

drivers/scsi/Kconfig | 5 +++--
drivers/scsi/scsi_scan.c | 5 +----
2 files changed, 4 insertions(+), 6 deletions(-)

---

diff -ruNp a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
--- a/drivers/scsi/Kconfig 2007-05-10 23:19:32.000000000 +0530
+++ b/drivers/scsi/Kconfig 2007-05-14 17:37:27.000000000 +0530
@@ -243,9 +243,10 @@ config SCSI_SCAN_ASYNC

config SCSI_WAIT_SCAN
tristate
- default m
depends on SCSI
- depends on MODULES
+ default y if SCSI=y
+ default m if SCSI=m
+ default n

menu "SCSI Transports"
depends on SCSI
diff -ruNp a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c 2007-05-14 16:06:43.000000000 +0530
+++ b/drivers/scsi/scsi_scan.c 2007-05-14 16:10:34.000000000 +0530
@@ -184,14 +184,11 @@ int scsi_complete_async_scans(void)
/* Only exported for the benefit of scsi_wait_scan */
EXPORT_SYMBOL_GPL(scsi_complete_async_scans);

-#ifndef MODULE
/*
* For async scanning we need to wait for all the scans to complete before
* trying to mount the root fs. Otherwise non-modular drivers may not be ready
- * yet.
+ * yet. This is done by scsi_wait_scan.
*/
-late_initcall(scsi_complete_async_scans);
-#endif

/**
* scsi_unlock_floptical - unlock device via a special MODE SENSE command

2007-05-14 12:23:59

by Satyam Sharma

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

> Hello,
>
> > > [...]
> > > config SCSI_WAIT_SCAN
> > > tristate
> > > - default m
> > > - depends on SCSI
> > > - depends on MODULES
> > > + default m if SCSI=m
> > > + default n
> >
> > Note that this also means SCSI_WAIT_SCAN=n (will not get compiled
> > and built even as a module) if SCSI=y. But this is perfectly fine, because
> > I see:
> >
> > #ifndef MODULE
> > late_initcall(scsi_complete_async_scans);
> > #endif
> >
> > in drivers/scsi/scsi_scan.c anyway, so the async scans will be waited
> > upon and get complete even when SCSI=y and scsi_scan_type=async,
> > i.e. you don't really need the scsi_wait_scan module in that case anyway.
>
> It seems there is a:
>
> late_initcall(wait_scan_init);
>
> in drivers/scsi/scsi_wait_scan.c too. So we can get rid of the redundant:
>
> #ifndef MODULE
> late_initcal(scsi_complete_async_scans);
> #endif
>
> in drivers/scsi/scsi_scan.c by enforcing SCSI_WAIT_SCAN=y when
> SCSI=y (and =m when SCSI=m).
>
> I guess this is probably the behaviour that James wanted originally?
>
> Anyway, attached patch (subsumes previous one) cleans up all this.
> Now, scsi_wait_scan is the only guy who controls waiting upon async
> scans to complete:
>
> If SCSI=n, SCSI_WAIT_SCAN=n, obviously.
>
> If SCSI=y, SCSI_WAIT_SCAN=y, so it gets built-in and is run at
> late_initcall.
>
> If SCSI=m, SCSI_WAIT_SCAN=m too and would hopefully be run
> by the initrd/initramfs scripts to wait for async SCSI bus scans to
> finish before allowing the boot to proceed.
>
> [ Attached patch does _not_ expose SCSI_WAIT_SCAN to the
> user at kernel configuration time, so if somebody wants it to be
> built as a module even when SCSI=y (why would anybody
> want that, doesn't make much sense to me to modprobe
> scsi_wait_scan _after_ boot-up), it would not be possible to
> do so. But if someone really wants that, let me know, we can
> add a depends, tristate "..." and help in this Kconfig option to
> accomplish that too. ]

This has sadly become a one-person thread, but Robert informs
me in private mail that we can further clean/simplify the patch:

[subsumes both previous patches]

Signed-off-by: Satyam Sharma <[email protected]>

---

drivers/scsi/Kconfig | 3 +--
drivers/scsi/scsi_scan.c | 5 +----
2 files changed, 2 insertions(+), 6 deletions(-)

---

diff -ruNp a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
--- a/drivers/scsi/Kconfig 2007-05-10 23:19:32.000000000 +0530
+++ b/drivers/scsi/Kconfig 2007-05-14 17:59:51.000000000 +0530
@@ -243,9 +243,8 @@ config SCSI_SCAN_ASYNC

config SCSI_WAIT_SCAN
tristate
- default m
depends on SCSI
- depends on MODULES
+ default SCSI

menu "SCSI Transports"
depends on SCSI
diff -ruNp a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
--- a/drivers/scsi/scsi_scan.c 2007-05-14 16:06:43.000000000 +0530
+++ b/drivers/scsi/scsi_scan.c 2007-05-14 16:10:34.000000000 +0530
@@ -184,14 +184,11 @@ int scsi_complete_async_scans(void)
/* Only exported for the benefit of scsi_wait_scan */
EXPORT_SYMBOL_GPL(scsi_complete_async_scans);

-#ifndef MODULE
/*
* For async scanning we need to wait for all the scans to complete before
* trying to mount the root fs. Otherwise non-modular drivers may not be ready
- * yet.
+ * yet. This is done by scsi_wait_scan.
*/
-late_initcall(scsi_complete_async_scans);
-#endif

/**
* scsi_unlock_floptical - unlock device via a special MODE SENSE command

2007-05-14 14:31:59

by James Bottomley

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Mon, 2007-05-14 at 17:53 +0530, Satyam Sharma wrote:
> > I guess this is probably the behaviour that James wanted originally?

No ... you're still not reading the explanation in the thread:

The wait scan module is designed to wait for scans of driver modules.
Whether SCSI=y or m has no effect on this ... you can still have modular
drivers with built in SCSI.

> > Anyway, attached patch (subsumes previous one) cleans up all this.
> > Now, scsi_wait_scan is the only guy who controls waiting upon async
> > scans to complete:
> >
> > If SCSI=n, SCSI_WAIT_SCAN=n, obviously.
> >
> > If SCSI=y, SCSI_WAIT_SCAN=y, so it gets built-in and is run at
> > late_initcall.
> >
> > If SCSI=m, SCSI_WAIT_SCAN=m too and would hopefully be run
> > by the initrd/initramfs scripts to wait for async SCSI bus scans to
> > finish before allowing the boot to proceed.
> >
> > [ Attached patch does _not_ expose SCSI_WAIT_SCAN to the
> > user at kernel configuration time, so if somebody wants it to be
> > built as a module even when SCSI=y (why would anybody
> > want that, doesn't make much sense to me to modprobe
> > scsi_wait_scan _after_ boot-up), it would not be possible to
> > do so. But if someone really wants that, let me know, we can
> > add a depends, tristate "..." and help in this Kconfig option to
> > accomplish that too. ]
>
> This has sadly become a one-person thread, but Robert informs
> me in private mail that we can further clean/simplify the patch:
>
> [subsumes both previous patches]
>
> Signed-off-by: Satyam Sharma <[email protected]>
>
> ---
>
> drivers/scsi/Kconfig | 3 +--
> drivers/scsi/scsi_scan.c | 5 +----
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> ---
>
> diff -ruNp a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> --- a/drivers/scsi/Kconfig 2007-05-10 23:19:32.000000000 +0530
> +++ b/drivers/scsi/Kconfig 2007-05-14 17:59:51.000000000 +0530
> @@ -243,9 +243,8 @@ config SCSI_SCAN_ASYNC
>
> config SCSI_WAIT_SCAN
> tristate
> - default m
> depends on SCSI
> - depends on MODULES
> + default SCSI
>
> menu "SCSI Transports"
> depends on SCSI
> diff -ruNp a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> --- a/drivers/scsi/scsi_scan.c 2007-05-14 16:06:43.000000000 +0530
> +++ b/drivers/scsi/scsi_scan.c 2007-05-14 16:10:34.000000000 +0530
> @@ -184,14 +184,11 @@ int scsi_complete_async_scans(void)
> /* Only exported for the benefit of scsi_wait_scan */
> EXPORT_SYMBOL_GPL(scsi_complete_async_scans);
>
> -#ifndef MODULE
> /*
> * For async scanning we need to wait for all the scans to complete before
> * trying to mount the root fs. Otherwise non-modular drivers may not be ready
> - * yet.
> + * yet. This is done by scsi_wait_scan.
> */
> -late_initcall(scsi_complete_async_scans);
> -#endif

This is actually not in mainline. It's an incorrect patch in -mm. The
problem with late_initcall is that they queue in link order ... so any
SCSI module (and there a few) linked after this won't benefit from it.
The obvious hack is to have a separate file with this in the final link.
However, Matthew Wilcox thinks he can do better.

James


2007-05-14 17:33:39

by Jan Engelhardt

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?


On May 13 2007 12:48, James Bottomley wrote:
>
>> Why does ATA select SCSI anyway? Surely PATA doesn't require it?
>
>That's a bit offtopic and to the wrong list.
>
>libata-pata does require SCSI ...

And in the long run, that SCSI parts which are actually used by ATA
should be factored out so that SCSI really is SCSI again, and not
"Common layer for SCSI, SATA and PATA" or so.

Jan
--

2007-05-14 18:42:38

by Alan

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Mon, 14 May 2007 19:29:12 +0200 (MEST)
Jan Engelhardt <[email protected]> wrote:

>
> On May 13 2007 12:48, James Bottomley wrote:
> >
> >> Why does ATA select SCSI anyway? Surely PATA doesn't require it?
> >
> >That's a bit offtopic and to the wrong list.
> >
> >libata-pata does require SCSI ...
>
> And in the long run, that SCSI parts which are actually used by ATA
> should be factored out so that SCSI really is SCSI again, and not
> "Common layer for SCSI, SATA and PATA" or so.

The common layer for the queueing is one thing, but the ATAPI devices
(CD-ROM etc) are SCSI commands over an ATA bus. A subset of SCSI commands
badly over an ATA bus but SCSI commands nevertheless - so they have the
same basic dependancies as USB storage does.

Alan

2007-05-14 20:11:50

by Jan Engelhardt

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?


On May 14 2007 19:46, Alan Cox wrote:
>> On May 13 2007 12:48, James Bottomley wrote:
>> >
>> >> Why does ATA select SCSI anyway? Surely PATA doesn't require it?
>> >That's a bit offtopic and to the wrong list.
>> >libata-pata does require SCSI ...
>>
>> And in the long run, that SCSI parts which are actually used by ATA
>> should be factored out so that SCSI really is SCSI again, and not
>> "Common layer for SCSI, SATA and PATA" or so.
>
>The common layer for the queueing is one thing, but the ATAPI devices
>(CD-ROM etc) are SCSI commands over an ATA bus. A subset of SCSI commands
>badly over an ATA bus but SCSI commands nevertheless - so they have the
>same basic dependancies as USB storage does.

Hm yes of course, but harddisks themselves do not normally need ATAPI;
so it would be cool to be capable of throwing it out for PATA-and-no-CDROM
boxes.


Jan
--

2007-05-15 00:41:18

by Satyam Sharma

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On 5/14/07, James Bottomley <[email protected]> wrote:
> On Mon, 2007-05-14 at 17:53 +0530, Satyam Sharma wrote:
> > > I guess this is probably the behaviour that James wanted originally?
>
> No ... you're still not reading the explanation in the thread:
>
> The wait scan module is designed to wait for scans of driver modules.
> Whether SCSI=y or m has no effect on this ... you can still have modular
> drivers with built in SCSI.

Ah, I see why we _want_ this built as a _module_ only, and don't even
want to expose the Kconfig option to the user, lest he screw himself later.
But dangling "default m"'s or "default y"'s not exposed to the user do
stand out discomfortingly in Kconfigs, wish there was a better way to
handle this.

> > > Anyway, attached patch (subsumes previous one) cleans up all this.
> > > Now, scsi_wait_scan is the only guy who controls waiting upon async
> > > scans to complete:
> > >
> > > If SCSI=n, SCSI_WAIT_SCAN=n, obviously.
> > >
> > > If SCSI=y, SCSI_WAIT_SCAN=y, so it gets built-in and is run at
> > > late_initcall.
> > >
> > > If SCSI=m, SCSI_WAIT_SCAN=m too and would hopefully be run
> > > by the initrd/initramfs scripts to wait for async SCSI bus scans to
> > > finish before allowing the boot to proceed.
> > >
> > > [ Attached patch does _not_ expose SCSI_WAIT_SCAN to the
> > > user at kernel configuration time, so if somebody wants it to be
> > > built as a module even when SCSI=y (why would anybody
> > > want that, doesn't make much sense to me to modprobe
> > > scsi_wait_scan _after_ boot-up), it would not be possible to
> > > do so. But if someone really wants that, let me know, we can
> > > add a depends, tristate "..." and help in this Kconfig option to
> > > accomplish that too. ]
> >
> > This has sadly become a one-person thread, but Robert informs
> > me in private mail that we can further clean/simplify the patch:
> >
> > [subsumes both previous patches]
> >
> > Signed-off-by: Satyam Sharma <[email protected]>
> >
> > ---
> >
> > drivers/scsi/Kconfig | 3 +--
> > drivers/scsi/scsi_scan.c | 5 +----
> > 2 files changed, 2 insertions(+), 6 deletions(-)
> >
> > ---
> >
> > diff -ruNp a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> > --- a/drivers/scsi/Kconfig 2007-05-10 23:19:32.000000000 +0530
> > +++ b/drivers/scsi/Kconfig 2007-05-14 17:59:51.000000000 +0530
> > @@ -243,9 +243,8 @@ config SCSI_SCAN_ASYNC
> >
> > config SCSI_WAIT_SCAN
> > tristate
> > - default m
> > depends on SCSI
> > - depends on MODULES
> > + default SCSI
> >
> > menu "SCSI Transports"
> > depends on SCSI
> > diff -ruNp a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> > --- a/drivers/scsi/scsi_scan.c 2007-05-14 16:06:43.000000000 +0530
> > +++ b/drivers/scsi/scsi_scan.c 2007-05-14 16:10:34.000000000 +0530
> > @@ -184,14 +184,11 @@ int scsi_complete_async_scans(void)
> > /* Only exported for the benefit of scsi_wait_scan */
> > EXPORT_SYMBOL_GPL(scsi_complete_async_scans);
> >
> > -#ifndef MODULE
> > /*
> > * For async scanning we need to wait for all the scans to complete before
> > * trying to mount the root fs. Otherwise non-modular drivers may not be ready
> > - * yet.
> > + * yet. This is done by scsi_wait_scan.
> > */
> > -late_initcall(scsi_complete_async_scans);
> > -#endif
>
> This is actually not in mainline. It's an incorrect patch in -mm. The
> problem with late_initcall is that they queue in link order ... so any
> SCSI module (and there a few) linked after this won't benefit from it.
> The obvious hack is to have a separate file with this in the final link.
> However, Matthew Wilcox thinks he can do better.

Yeah, thanks for this explanation. -mm is probably what confused me
here. I did consider the fact that late_initcall's are called in link order,
but saw that there was an explicit effort (hack?) in drivers/scsi/Makefile
to keep scsi_wait_scan last so that SCSI devices probe earlier ... not
that this helps us with modular drivers, of course.

Thanks,
Satyam

2007-05-15 11:26:47

by Simon Arlott

[permalink] [raw]
Subject: Re: why does x86 "make defconfig" build a single, lonely module?

On Tue, May 15, 2007 01:41, Satyam Sharma wrote:
> On 5/14/07, James Bottomley <[email protected]> wrote:
>> On Mon, 2007-05-14 at 17:53 +0530, Satyam Sharma wrote:
>> > > I guess this is probably the behaviour that James wanted originally?
>>
>> No ... you're still not reading the explanation in the thread:
>>
>> The wait scan module is designed to wait for scans of driver modules.
>> Whether SCSI=y or m has no effect on this ... you can still have modular
>> drivers with built in SCSI.
>
> Ah, I see why we _want_ this built as a _module_ only, and don't even
> want to expose the Kconfig option to the user, lest he screw himself later.
> But dangling "default m"'s or "default y"'s not exposed to the user do
> stand out discomfortingly in Kconfigs, wish there was a better way to
> handle this.

I've already suggested a sysfs attribute - or something equivalent - would
be much better. It's just one function that a user might want to run multiple
times (e.g. after adding scsi devices?) - why should loading a module be used
for this?

--
Simon Arlott

2007-05-15 12:02:43

by Matthew Wilcox

[permalink] [raw]
Subject: Asynchronous scsi scanning

On Tue, May 15, 2007 at 12:26:29PM +0100, Simon Arlott wrote:
> I've already suggested a sysfs attribute - or something equivalent - would
> be much better. It's just one function that a user might want to run multiple
> times (e.g. after adding scsi devices?) - why should loading a module be used
> for this?

It's easy to suggest a sysfs attribute. What you've failed to do is
suggest the pathname of the sysfs attribute, the contents of it, or the
semantics of it (read-only? read-write? write-only? blocking?)

My personal favourite would be to add a new verb to /proc/scsi/scsi, but
James dislikes that idea.

I'd *really* like to hear from distro people. What is the most
convenient way for you to implement "load all the scsi modules, then
wait until all devices are found"? James and I had thought that loading
a new module would be the easiest way for you, but it seems inconvenient
for you.

2007-05-15 16:31:18

by Simon Arlott

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On 15/05/07 13:02, Matthew Wilcox wrote:
> On Tue, May 15, 2007 at 12:26:29PM +0100, Simon Arlott wrote:
>> I've already suggested a sysfs attribute - or something equivalent - would
>> be much better. It's just one function that a user might want to run multiple
>> times (e.g. after adding scsi devices?) - why should loading a module be used
>> for this?
>
> It's easy to suggest a sysfs attribute. What you've failed to do is
> suggest the pathname of the sysfs attribute, the contents of it, or the
> semantics of it (read-only? read-write? write-only? blocking?)

I would assume that should be up to SCSI users/maintainer(s). The only thing
I use the SCSI driver for is usb-storage/ATAPI.

> My personal favourite would be to add a new verb to /proc/scsi/scsi, but
> James dislikes that idea.
>
> I'd *really* like to hear from distro people. What is the most
> convenient way for you to implement "load all the scsi modules, then
> wait until all devices are found"? James and I had thought that loading
> a new module would be the easiest way for you, but it seems inconvenient
> for you.

It's inconvenient for people who *don't* use it to be unable to stop the
module being built and installed.

--
Simon Arlott

2007-05-15 17:29:20

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Tue, May 15, 2007 at 05:30:50PM +0100, Simon Arlott wrote:
> On 15/05/07 13:02, Matthew Wilcox wrote:
> >It's easy to suggest a sysfs attribute. What you've failed to do is
> >suggest the pathname of the sysfs attribute, the contents of it, or the
> >semantics of it (read-only? read-write? write-only? blocking?)
>
> I would assume that should be up to SCSI users/maintainer(s). The only
> thing I use the SCSI driver for is usb-storage/ATAPI.

Then you're not so much "suggesting a sysfs attribute" as whining.

> >I'd *really* like to hear from distro people. What is the most
> >convenient way for you to implement "load all the scsi modules, then
> >wait until all devices are found"? James and I had thought that loading
> >a new module would be the easiest way for you, but it seems inconvenient
> >for you.
>
> It's inconvenient for people who *don't* use it to be unable to stop the
> module being built and installed.

Why? You're not forced to load the module. In what way does it
inconvenience you? Nobody's making you run 'make modules_install'.
I often forget to myself.

2007-05-15 21:57:02

by Stefan Richter

[permalink] [raw]
Subject: [PATCH] SCSI: Let users disable SCSI_WAIT_SCAN to be built

Signed-off-by: Stefan Richter <[email protected]>
---
drivers/scsi/Kconfig | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

Index: linux-2.6.22-rc1/drivers/scsi/Kconfig
===================================================================
--- linux-2.6.22-rc1.orig/drivers/scsi/Kconfig
+++ linux-2.6.22-rc1/drivers/scsi/Kconfig
@@ -241,11 +241,19 @@ config SCSI_SCAN_ASYNC
You can override this choice by specifying "scsi_mod.scan=sync"
or async on the kernel's command line.

-config SCSI_WAIT_SCAN
+config SCSI_WAIT_SCAN_NO_Y
tristate
default m
- depends on SCSI
- depends on MODULES
+
+config SCSI_WAIT_SCAN
+ tristate "Pseudo driver which waits for SCSI scanning to finish"
+ depends on MODULES && SCSI && SCSI_WAIT_SCAN_NO_Y
+ help
+ When loaded, this module will do nothing else than wait for
+ SCSI low-level drivers to finish asynchronous scanning.
+ The module will be called scsi_wait_scan.
+
+ Most people can say n here.

menu "SCSI Transports"
depends on SCSI


Is there a better way to restrict SCSI_WAIT_SCAN to m and n?
--
Stefan Richter
-=====-=-=== -=-= -====
http://arcgraph.de/sr/

2007-05-15 23:28:10

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

Hi,

[ I appreciate you forked the thread and gave it a better subject name,
it would be better still if you could maintain the original CC list, thanks. ]

On Tue, May 15, 2007 at 12:26:29PM +0100, Simon Arlott wrote:
> I've already suggested a sysfs attribute - or something equivalent - would
> be much better. It's just one function that a user might want to run multiple
> times (e.g. after adding scsi devices?) - why should loading a module be used
> for this?

I have to agree with Simon that ...

static int __init wait_scan_init(void)
{
scsi_complete_async_scans();
return 0;
/* BTW this could've been return scsi_complete_async_scans();
* I see scsi_complete_async_scans() never fails, but still. */
}
late_initcall(wait_scan_init);

... does _not_ deserve to be a module, and writing/building a module
for something like this (just to run a function in some kernel subsytem)
does not seem to be the proper way to solve the problem either. And
IMHO sysfs attribute for this is a good (well, better than this module
thing, at least) suggestion -- I hope you'd take it more seriously.

On 5/15/07, Matthew Wilcox <[email protected]> wrote:
> > >It's easy to suggest a sysfs attribute. What you've failed to do is
> > >suggest the pathname of the sysfs attribute,

/sys/module/scsi_mod/parameters/wait_for_async_scans (?)
Doesn't really matter, but perhaps who created the sysfs namespace
for scsi in /sys/module/scsi_mod/... could be the best person to suggest.

> > >the contents of it, or the

Merely needs to be a "echo 1 > " kind of attribute. Whenever a
store_ happens on that attribute (and it's "1"), we just call
scsi_complete_async_scans(). Then we reset the attribute itself back
to 0. Also we could introduce a lock on wait_for_async_scans so that
we don't call scsi_complete_async_scans() twice if someone
accidentally "echo 1 > "'s to it more than once (if that would really be
a bad thing, otherwise it's fine). Also, something like ... if you read the
attribute _during_ the scsi_complete_async_scans(), then you get to
see "1".

All this is doesn't really matter, anyway, all that we really care about
is that: scsi_complete_async_scans() should run whenever something
gets written to this attribute, isn't it?

> > >semantics of it (read-only? read-write? write-only?

Well, it _has_ to be write, don't really care if it's read-write or
write-only. I would still prefer read-write, but we can go ahead with
write-only too. It doesn't really matter, does it?

> > >blocking?)

Why, blocking, of course. scsi_complete_async_scans() by definition
is supposed to _wait_ for async scans to complete, after all? And I
see a wait_for_completion() in their which means the current method
will also block during insmod time anyway, so we'll only be
maintaining present behaviour.

> > >I'd *really* like to hear from distro people. What is the most
> > >convenient way for you to implement "load all the scsi modules, then
> > >wait until all devices are found"?

What do the distro people really care?

It's either going to be a:

modprobe scsi_wait_scan

versus a:

echo 1 > /sys/module/scsi_mod/.../wait_for_async_scans

somewhere in some script. In fact, the latter method seems simpler,
saner, better (in every which way)!

> > >James and I had thought that loading
> > >a new module would be the easiest way for you, but it seems inconvenient
> > >for you.

It's not _inconvenient_. Just that writing/building a module for accomplishing
something like that ... is just not _right_.

> > It's inconvenient for people who *don't* use it to be unable to stop the
> > module being built and installed.

Yes.

> Why? You're not forced to load the module. In what way does it
> inconvenience you? Nobody's making you run 'make modules_install'.
> I often forget to myself.

OK, I'll get really silly here myself. I don't want even that half a second of
overhead when that module is being _built_ (during make modules), not
the overhead of copying / installing at modules_install time.

I apologize if I sounded impolite, and I certainly don't want to act
demanding / difficult or anything, but it's just that doing this via a sysfs
attribute (or hey, anything else!) sounds a better way to tackle this than
this module thing. IMHO, at least.

Satyam

2007-05-15 23:29:58

by Arjan van de Ven

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

Satyam Sharma wrote:
>> > >semantics of it (read-only? read-write? write-only?
>
> Well, it _has_ to be write, don't really care if it's read-write or
> write-only. I would still prefer read-write, but we can go ahead with
> write-only too. It doesn't really matter, does it?

just to be devils advocate...
it should be a read that returns when done, and that can be polled

2007-05-15 23:49:51

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On 5/16/07, Arjan van de Ven <[email protected]> wrote:
> Satyam Sharma wrote:
> >> > >semantics of it (read-only? read-write? write-only?
> >
> > Well, it _has_ to be write, don't really care if it's read-write or
> > write-only. I would still prefer read-write, but we can go ahead with
> > write-only too. It doesn't really matter, does it?
>
> just to be devils advocate...
> it should be a read that returns when done,

Heh, yeah. We just need to trigger that scsi_complete_async_scans()
after all ... might as well abuse all intuition on the user's behalf :-)

> and that can be polled

Gaah! :-)

But seriously, though, this sysfs attribute can be implemented
_any which way_. Better for us if we do it the simplest way (and which
taxes the user's intuition the least). Just that Matthew asked so many
questions so I thought I might as well answer them :-)

2007-05-16 02:51:33

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Wed, May 16, 2007 at 04:57:52AM +0530, Satyam Sharma wrote:
> [ I appreciate you forked the thread and gave it a better subject name,
> it would be better still if you could maintain the original CC list,
> thanks. ]

I removed the people I didn't think needed to be on the Cc list any more,
since I was changing the direction of the thread.

> On 5/15/07, Matthew Wilcox <[email protected]> wrote:
> >> >It's easy to suggest a sysfs attribute. What you've failed to do is
> >> >suggest the pathname of the sysfs attribute,
>
> /sys/module/scsi_mod/parameters/wait_for_async_scans (?)
> Doesn't really matter, but perhaps who created the sysfs namespace
> for scsi in /sys/module/scsi_mod/... could be the best person to suggest.

No, it does matter. Your suggestion doesn't work, because
/sys/module/scsi_mod/parameters/ belongs to the module code. To create
a new attribute there, you use the module_param() code -- and there's
no way to have code called when your parameter is changed.

> >Why? You're not forced to load the module. In what way does it
> >inconvenience you? Nobody's making you run 'make modules_install'.
> >I often forget to myself.
>
> OK, I'll get really silly here myself. I don't want even that half a second
> of
> overhead when that module is being _built_ (during make modules), not
> the overhead of copying / installing at modules_install time.

You're claiming that 0.7 second (I just timed it on a 3 year old
laptop) *inconveniences* you?

> I apologize if I sounded impolite, and I certainly don't want to act
> demanding / difficult or anything, but it's just that doing this via a sysfs
> attribute (or hey, anything else!) sounds a better way to tackle this than
> this module thing. IMHO, at least.

This whole thing is such a tempest in a teapot. I really don't
understand why you care so much.

2007-05-16 03:00:05

by Roland Dreier

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

> No, it does matter. Your suggestion doesn't work, because
> /sys/module/scsi_mod/parameters/ belongs to the module code. To create
> a new attribute there, you use the module_param() code -- and there's
> no way to have code called when your parameter is changed.

If I'm not misunderstanding what you're talking about, there is
actually a way to have code called when a module parameter is changed:
module_param_call().

- R.

2007-05-16 14:43:29

by Stefan Richter

[permalink] [raw]
Subject: Re: [PATCH] SCSI: Let users disable SCSI_WAIT_SCAN to be built

I wrote:
> --- linux-2.6.22-rc1.orig/drivers/scsi/Kconfig
> +++ linux-2.6.22-rc1/drivers/scsi/Kconfig
> @@ -241,11 +241,19 @@ config SCSI_SCAN_ASYNC
> You can override this choice by specifying "scsi_mod.scan=sync"
> or async on the kernel's command line.
>
> -config SCSI_WAIT_SCAN
> +config SCSI_WAIT_SCAN_NO_Y
> tristate
> default m
> - depends on SCSI
> - depends on MODULES

SCSI_WAIT_SCAN_NO_Y is unnecessary, as Randy Dunlap pointed out in "Re:
How to force Kconfig tristate into range n..m?".
http://lkml.org/lkml/2007/5/15/320

> +
> +config SCSI_WAIT_SCAN
> + tristate "Pseudo driver which waits for SCSI scanning to finish"
> + depends on MODULES && SCSI && SCSI_WAIT_SCAN_NO_Y

depends on SCSI && m

> + help
> + When loaded, this module will do nothing else than wait for
> + SCSI low-level drivers to finish asynchronous scanning.
> + The module will be called scsi_wait_scan.

There should be explained that it is the command "modprobe
scsi_wait_scan" which is doing the waiting, and that this is useful or
required in initrds.

> + Most people can say n here.

This sentence should probably be omitted, as it may be wrong in the
future and unsafe already now.

I will resend an updated patch.
--
Stefan Richter
-=====-=-=== -=-= =----
http://arcgraph.de/sr/

2007-05-17 14:00:29

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH] SCSI: Let users disable SCSI_WAIT_SCAN to be built

On Wed, 2007-05-16 at 16:43 +0200, Stefan Richter wrote:
> I wrote:
> > --- linux-2.6.22-rc1.orig/drivers/scsi/Kconfig
> > +++ linux-2.6.22-rc1/drivers/scsi/Kconfig
> > @@ -241,11 +241,19 @@ config SCSI_SCAN_ASYNC
> > You can override this choice by specifying "scsi_mod.scan=sync"
> > or async on the kernel's command line.
> >
> > -config SCSI_WAIT_SCAN
> > +config SCSI_WAIT_SCAN_NO_Y
> > tristate
> > default m
> > - depends on SCSI
> > - depends on MODULES
>
> SCSI_WAIT_SCAN_NO_Y is unnecessary, as Randy Dunlap pointed out in "Re:
> How to force Kconfig tristate into range n..m?".
> http://lkml.org/lkml/2007/5/15/320
>
> > +
> > +config SCSI_WAIT_SCAN
> > + tristate "Pseudo driver which waits for SCSI scanning to finish"
> > + depends on MODULES && SCSI && SCSI_WAIT_SCAN_NO_Y
>
> depends on SCSI && m
>
> > + help
> > + When loaded, this module will do nothing else than wait for
> > + SCSI low-level drivers to finish asynchronous scanning.
> > + The module will be called scsi_wait_scan.
>
> There should be explained that it is the command "modprobe
> scsi_wait_scan" which is doing the waiting, and that this is useful or
> required in initrds.
>
> > + Most people can say n here.
>
> This sentence should probably be omitted, as it may be wrong in the
> future and unsafe already now.
>
> I will resend an updated patch.

Please don't bother ... I really want a more considered way of fixing
this. If everyone decides the best way is exposing this to the user,
then this is the way to do it ... however, I still don't consider this
argument made out yet.

James

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index d28c14e..a6b95cd 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -242,10 +242,14 @@ config SCSI_SCAN_ASYNC
or async on the kernel's command line.

config SCSI_WAIT_SCAN
- tristate
- default m
+ tristate "Build Scan Wait Module"
+ depends on m
depends on SCSI
depends on MODULES
+ help
+ The wait scan module builds a module which is used by
+ initramdisk boots to wait for scans to complete after
+ all SCSI modules have been loaded. If unsure, say M here

menu "SCSI Transports"
depends on SCSI


2007-05-17 17:02:42

by Satyam Sharma

[permalink] [raw]
Subject: Re: [PATCH] SCSI: Let users disable SCSI_WAIT_SCAN to be built

On 5/17/07, James Bottomley <[email protected]> wrote:
> [...]
> Please don't bother ... I really want a more considered way of fixing
> this. If everyone decides the best way is exposing this to the user,
> then this is the way to do it ... however, I still don't consider this
> argument made out yet.

Neither do I, considering that the user can switch async scanning on
at any later time with the scan=async module option. The absence
of the async scan wait module will only screw things up for him later.

But then I don't consider an argument made out yet for doing this
via a separate module in the first place. scsi_wait_scan just does
_not_ want/deserve/need to be a module.

Of course, whining is useless, and discussions on lkml tend to be
most constructive / progressive only over code, so I'll try and see
if I can learn enough of the code in there to fix a patch to do the
async scan waiting thing using some other mechanism; this
module thing seems just horribly wrong/unnecessary.

2007-05-17 17:13:22

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

Hi Matthew,

On 5/16/07, Matthew Wilcox <[email protected]> wrote:
> [...]
> > /sys/module/scsi_mod/parameters/wait_for_async_scans (?)
> > Doesn't really matter, but perhaps who created the sysfs namespace
> > for scsi in /sys/module/scsi_mod/... could be the best person to suggest.
>
> No, it does matter. Your suggestion doesn't work, because
> /sys/module/scsi_mod/parameters/ belongs to the module code. To create
> a new attribute there, you use the module_param() code -- and there's
> no way to have code called when your parameter is changed.

Ok, thanks for pointing out that /sys/module/scsi_mod/parameters/wait...
is _wrong_. Could you suggest something that would be _right_?

> > OK, I'll get really silly here myself. I don't want even that half a second
> > of
> > overhead when that module is being _built_ (during make modules), not
> > the overhead of copying / installing at modules_install time.
>
> You're claiming that 0.7 second (I just timed it on a 3 year old
> laptop) *inconveniences* you?

...

On 5/16/07, Satyam Sharma <[email protected]> wrote:
OK, I'll get really silly here myself. ...

...

On 5/16/07, Satyam Sharma <[email protected]> wrote:
It's not _inconvenient_. Just that writing/building a module for accomplishing
something like that ... is just not _right_.

...

On 5/16/07, Satyam Sharma <[email protected]> wrote:
static int __init wait_scan_init(void)
{
scsi_complete_async_scans();
return 0;
/* BTW this could've been return scsi_complete_async_scans();
* I see scsi_complete_async_scans() never fails, but still. */
}
late_initcall(wait_scan_init);
... does _not_ deserve to be a module, and writing/building a module
for something like this (just to run a function in some kernel subsytem)
does not seem to be the proper way to solve the problem either.

...

> This whole thing is such a tempest in a teapot. I really don't
> understand why you care so much.

You're almost right here. But IMHO this is simply a case of
doing something in some kernel subsystem in a proper/better
way than it is being done presently.

Anyway, like I said on another thread, discussions here tend to be
most productive only over code, so I'll try and make a patch to do
this some other way.

Thanks,
Satyam

2007-05-17 17:20:34

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Thu, May 17, 2007 at 10:43:06PM +0530, Satyam Sharma wrote:
> >No, it does matter. Your suggestion doesn't work, because
> >/sys/module/scsi_mod/parameters/ belongs to the module code. To create
> >a new attribute there, you use the module_param() code -- and there's
> >no way to have code called when your parameter is changed.

(thanks, Roland for pointing out that I'm incorrect about code being
called)

> Ok, thanks for pointing out that /sys/module/scsi_mod/parameters/wait...
> is _wrong_. Could you suggest something that would be _right_?

No, I can't, which is why I find it hard to like the idea of "use
sysfs". I have no particular love for using a module like this, but
my preferred way (a new verb for /proc/scsi/scsi) isn't liked by others.
So here we stand.

> You're almost right here. But IMHO this is simply a case of
> doing something in some kernel subsystem in a proper/better
> way than it is being done presently.
>
> Anyway, like I said on another thread, discussions here tend to be
> most productive only over code, so I'll try and make a patch to do
> this some other way.

Come up with a sensible suggestion, and I'll listen to you. Code isn't
the issue. API is the issue.

2007-05-17 17:33:21

by Benjamin LaHaise

[permalink] [raw]
Subject: sysfs makes scaling suck Re: Asynchronous scsi scanning

On Wed, May 16, 2007 at 04:57:52AM +0530, Satyam Sharma wrote:
>
> echo 1 > /sys/module/scsi_mod/.../wait_for_async_scans
>
> somewhere in some script. In fact, the latter method seems simpler,
> saner, better (in every which way)!

Please don't force sysfs on people. Just watch how it keels over and dies
when you end up with lots of disks/network interfaces on reasonably sized
boxes. 16 statically allocated files per network interface starts being
a problem once you've got a few thousand interfaces. Anything that forces
me to use sysfs is not gonna fly.

-ben
--
"Time is of no importance, Mr. President, only life is important."
Don't Email: <[email protected]>.

2007-05-17 17:41:43

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On 5/17/07, Matthew Wilcox <[email protected]> wrote:
> On Thu, May 17, 2007 at 10:43:06PM +0530, Satyam Sharma wrote:
> > >No, it does matter. Your suggestion doesn't work, because
> > >/sys/module/scsi_mod/parameters/ belongs to the module code. To create
> > >a new attribute there, you use the module_param() code -- and there's
> > >no way to have code called when your parameter is changed.
>
> (thanks, Roland for pointing out that I'm incorrect about code being
> called)

> Come up with a sensible suggestion, and I'll listen to you. Code isn't
> the issue. API is the issue.

Well, that itself is a suggestion.

> > Ok, thanks for pointing out that /sys/module/scsi_mod/parameters/wait...
> > is _wrong_. Could you suggest something that would be _right_?
>
> No, I can't, which is why I find it hard to like the idea of "use
> sysfs". I have no particular love for using a module like this, but
> my preferred way (a new verb for /proc/scsi/scsi) isn't liked by others.

Another command to /proc/scsi/scsi isn't a bad thought at all, considering
we're not _inventing_ a *new* /proc/not-related-to-processes interface, but
simply extending one that already exists. But then James / others are also
somewhat justified in shooting that down. I bet a lot of people would find
that even worse than this whole module affair.

2007-05-17 17:47:20

by James Bottomley

[permalink] [raw]
Subject: Re: sysfs makes scaling suck Re: Asynchronous scsi scanning

On Thu, 2007-05-17 at 13:32 -0400, Benjamin LaHaise wrote:
> On Wed, May 16, 2007 at 04:57:52AM +0530, Satyam Sharma wrote:
> >
> > echo 1 > /sys/module/scsi_mod/.../wait_for_async_scans
> >
> > somewhere in some script. In fact, the latter method seems simpler,
> > saner, better (in every which way)!
>
> Please don't force sysfs on people. Just watch how it keels over and dies
> when you end up with lots of disks/network interfaces on reasonably sized
> boxes. 16 statically allocated files per network interface starts being
> a problem once you've got a few thousand interfaces. Anything that forces
> me to use sysfs is not gonna fly.

Well, for that case, you just never enable async scanning

But also, the sysfs with over 4,000 (and higher) devices was
specifically checked by OSDL (actually as part of the CGL testing) some
of the Manoj changes (for unpinning entries etc) were needed to get it
to function, but as of now, I believe an enterprise scaling test works
reasonably well for it ... there certainly wasn't any evidence of it
dying horribly in the tests.

James


2007-05-17 17:50:28

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: sysfs makes scaling suck Re: Asynchronous scsi scanning

On Thu, May 17, 2007 at 01:45:24PM -0400, James Bottomley wrote:
> But also, the sysfs with over 4,000 (and higher) devices was
> specifically checked by OSDL (actually as part of the CGL testing) some
> of the Manoj changes (for unpinning entries etc) were needed to get it
> to function, but as of now, I believe an enterprise scaling test works
> reasonably well for it ... there certainly wasn't any evidence of it
> dying horribly in the tests.

i386 exhausts lowmem very quickly. SCSI is in a bit better shape than
network devices as the multiplier is only around 4 compared to 16 for network
devices.

My point stands, though. Forcing every new feature to go in via sysfs is
not the right thing to do. Some people don't/can't use it, please remember
them.

-ben
--
"Time is of no importance, Mr. President, only life is important."
Don't Email: <[email protected]>.

2007-05-17 18:24:44

by Christoph Hellwig

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Thu, May 17, 2007 at 11:11:10PM +0530, Satyam Sharma wrote:
> Another command to /proc/scsi/scsi isn't a bad thought at all, considering

Yes it is. /proc/scsi/scsi is a horrible interface and deprecated since
the start of the 2.6 series.

2007-05-17 18:47:55

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

Hi Christoph,

On 5/17/07, Christoph Hellwig <[email protected]> wrote:
> On Thu, May 17, 2007 at 11:11:10PM +0530, Satyam Sharma wrote:
> > Another command to /proc/scsi/scsi isn't a bad thought at all, considering
>
> Yes it is. /proc/scsi/scsi is a horrible interface and deprecated since
> the start of the 2.6 series.

What I have in mind is actually this: We implement commands to
trigger the scsi_complete_async_scans() using *both* the
(legacy, agreed) /proc/scsi/scsi interface _and_ using a sysfs
attribute for this (either in /sys/module/scsi_mod/parameter/, using
a module_param_call as Roland suggested, or, create a new
/sys/module/scsi_mod/<whatever>/wait_..., whatever is appropriate).

/proc/scsi/scsi is now deprecated _because_ the sysfs alternative
became available, after all (we couldn't pull functionality /
interfaces previously exposed to userspace from under them
without giving equivalent alternatives anyway).

However, Ben does have a point that we shouldn't force those
using SCSI (and wishing to use the new async scanning
feature) to depend on and use sysfs too, so those like him could
use /proc/scsi/scsi. The idea is that with time sysfs gets its act
together and thus becomes standard enough on all Linux boxen
(much like /proc is today), and those like Ben could later switch
to that.

2007-05-17 18:51:36

by Christoph Hellwig

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Fri, May 18, 2007 at 12:17:40AM +0530, Satyam Sharma wrote:
> However, Ben does have a point that we shouldn't force those
> using SCSI (and wishing to use the new async scanning
> feature) to depend on and use sysfs too

yes, we do. an no, procfs is a much worse filesystem to depend
on for drivers. if people don't want sysfs they can either do
the synchronous scan or do their own scan in userspace.

2007-05-17 19:04:53

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On 5/18/07, Christoph Hellwig <[email protected]> wrote:
> On Fri, May 18, 2007 at 12:17:40AM +0530, Satyam Sharma wrote:
> > However, Ben does have a point that we shouldn't force those
> > using SCSI (and wishing to use the new async scanning
> > feature) to depend on and use sysfs too
>
> yes, we do. an no, procfs is a much worse filesystem to depend
> on for drivers.

By depend on I meant being forced to build and run it. /proc is
clearly more-or-less standard that almost everybody uses, otoh
users that do not run sysfs are more common.

> if people don't want sysfs they can either do the synchronous scan
> or do their own scan in userspace.

Hmmm, actually those other users could easily write and maintain
a 20-line patch that does the wait for async scans thing for them
using /proc/scsi/scsi in any case.

2007-05-17 19:40:12

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Fri, May 18, 2007 at 12:34:40AM +0530, Satyam Sharma wrote:
> Hmmm, actually those other users could easily write and maintain
> a 20-line patch that does the wait for async scans thing for them
> using /proc/scsi/scsi in any case.

How about the three users who're bothered by this extra module being
built maintain a one-line patch to Kconfig and leave well enough alone?

2007-05-17 20:13:12

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Thu, May 17, 2007 at 01:39:54PM -0600, Matthew Wilcox wrote:
> On Fri, May 18, 2007 at 12:34:40AM +0530, Satyam Sharma wrote:
> > Hmmm, actually those other users could easily write and maintain
> > a 20-line patch that does the wait for async scans thing for them
> > using /proc/scsi/scsi in any case.
>
> How about the three users who're bothered by this extra module being
> built maintain a one-line patch to Kconfig and leave well enough alone?

The module has an added bonus that it doesn't require any new tools to
make work. Doing it via sysfs/procfs means a new rev of whatever tool
generates the boot initrd, plus fixing up boot scripts. Loading a module
can be done via a simple option to the existing boot tools.

-ben
--
"Time is of no importance, Mr. President, only life is important."
Don't Email: <[email protected]>.

2007-05-17 21:30:54

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Thu, May 17, 2007 at 03:43:26PM -0400, Benjamin LaHaise wrote:
> On Thu, May 17, 2007 at 01:39:54PM -0600, Matthew Wilcox wrote:
> > On Fri, May 18, 2007 at 12:34:40AM +0530, Satyam Sharma wrote:
> > > Hmmm, actually those other users could easily write and maintain
> > > a 20-line patch that does the wait for async scans thing for them
> > > using /proc/scsi/scsi in any case.
> >
> > How about the three users who're bothered by this extra module being
> > built maintain a one-line patch to Kconfig and leave well enough alone?
>
> The module has an added bonus that it doesn't require any new tools to
> make work. Doing it via sysfs/procfs means a new rev of whatever tool
> generates the boot initrd, plus fixing up boot scripts. Loading a module
> can be done via a simple option to the existing boot tools.

That was what James and I thought. However, the distros seem unhappy
with it. Of course, they won't actually *comment* on it, they just
disable the async scan and won't talk about why.

What's the point of this kernel-packagers list if not this kind of issue?

2007-05-17 21:42:51

by Dave Jones

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Thu, May 17, 2007 at 03:30:43PM -0600, Matthew Wilcox wrote:
> On Thu, May 17, 2007 at 03:43:26PM -0400, Benjamin LaHaise wrote:
> > On Thu, May 17, 2007 at 01:39:54PM -0600, Matthew Wilcox wrote:
> > > On Fri, May 18, 2007 at 12:34:40AM +0530, Satyam Sharma wrote:
> > > > Hmmm, actually those other users could easily write and maintain
> > > > a 20-line patch that does the wait for async scans thing for them
> > > > using /proc/scsi/scsi in any case.
> > >
> > > How about the three users who're bothered by this extra module being
> > > built maintain a one-line patch to Kconfig and leave well enough alone?
> >
> > The module has an added bonus that it doesn't require any new tools to
> > make work. Doing it via sysfs/procfs means a new rev of whatever tool
> > generates the boot initrd, plus fixing up boot scripts. Loading a module
> > can be done via a simple option to the existing boot tools.
>
> That was what James and I thought. However, the distros seem unhappy
> with it. Of course, they won't actually *comment* on it, they just
> disable the async scan and won't talk about why.

FWIW, Fedora 7 has it enabled, and afaik, Peter (mkinitrd maintainer) is happy
with the current situation. It's my understanding that the latest ubuntu
release has it enabled too, though obviously I can't speak for whether
or not they're happy with the status quo.

Dave

--
http://www.codemonkey.org.uk

2007-05-17 22:01:11

by Peter Jones

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

Dave Jones wrote:
> On Thu, May 17, 2007 at 03:30:43PM -0600, Matthew Wilcox wrote:
> > On Thu, May 17, 2007 at 03:43:26PM -0400, Benjamin LaHaise wrote:
> > > On Thu, May 17, 2007 at 01:39:54PM -0600, Matthew Wilcox wrote:
> > > > On Fri, May 18, 2007 at 12:34:40AM +0530, Satyam Sharma wrote:
> > > > > Hmmm, actually those other users could easily write and maintain
> > > > > a 20-line patch that does the wait for async scans thing for them
> > > > > using /proc/scsi/scsi in any case.

This is one of the things we currently do. There are problems with it,
not the least of which being that it's hard to know how long to wait,
and waiting excessively long generates user complaints.

> > > > How about the three users who're bothered by this extra module being
> > > > built maintain a one-line patch to Kconfig and leave well enough alone?

(I assume this is about scsi_wait_scan)

> > > The module has an added bonus that it doesn't require any new tools to
> > > make work. Doing it via sysfs/procfs means a new rev of whatever tool
> > > generates the boot initrd, plus fixing up boot scripts. Loading a module
> > > can be done via a simple option to the existing boot tools.

This isn't really true -- loading the module requires that a user is
actually running the tools and knows to do it, which is rarely (and
ideally never) the case. And frankly, every single one of our users
would have to do it.

So really, either way means we need to update the tools. It also
doesn't really solve the problem -- when I insert "usb-storage", the
SCSI scan may still finish while we're still enumerating the bus for USB
devices. (I'd be willing to believe I'm wrong about this specific
example, but I suspect the principle still applies for some other driver.)

In practice, we wind up doing the compare/timeout loop as on
/proc/scsi/scsi, but on /proc/bus/usb/devices or
/sys/bus/ieee1394/drivers/sbp2 instead.

> > That was what James and I thought. However, the distros seem unhappy
> > with it. Of course, they won't actually *comment* on it, they just
> > disable the async scan and won't talk about why.
>
> FWIW, Fedora 7 has it enabled, and afaik, Peter (mkinitrd maintainer) is happy
> with the current situation. It's my understanding that the latest ubuntu
> release has it enabled too, though obviously I can't speak for whether
> or not they're happy with the status quo.

I wouldn't say I'm *happy* with the current situation, but we're to the
point where it works for most users.

At the same time, we're moving towards polling on the hotplug socket,
waiting for specific devices to appear from which to build and mount
"/". That should obviate the need for much of this in most cases.

--
Peter

2007-05-17 22:24:48

by Peter Jones

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

Matthew Wilcox wrote:
> On Tue, May 15, 2007 at 12:26:29PM +0100, Simon Arlott wrote:
>> I've already suggested a sysfs attribute - or something equivalent - would
>> be much better. It's just one function that a user might want to run multiple
>> times (e.g. after adding scsi devices?) - why should loading a module be used
>> for this?
>
> It's easy to suggest a sysfs attribute. What you've failed to do is
> suggest the pathname of the sysfs attribute, the contents of it, or the
> semantics of it (read-only? read-write? write-only? blocking?)
>
> My personal favourite would be to add a new verb to /proc/scsi/scsi, but
> James dislikes that idea.
>
> I'd *really* like to hear from distro people. What is the most
> convenient way for you to implement "load all the scsi modules, then
> wait until all devices are found"? James and I had thought that loading
> a new module would be the easiest way for you, but it seems inconvenient
> for you.

Basically we're going to wind up listening for SUBSYSTEM=block hotplug
events. What would be helpful is if
/sys/block/$foo/device/{model,vendor} were populated when we get the
event (or, since there are good reasons why they're not, if we got
another event afterwards that said they'd been populated).

Even more helpful if there were also sysfs attributes for serial numbers
and such from INQUIRY VPD data. That would make probing for the
*correct* drive(s) much simpler.

--
Peter

2007-05-18 03:42:19

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

> On Thu, May 17, 2007 at 01:39:54PM -0600, Matthew Wilcox wrote:
> > On Fri, May 18, 2007 at 12:34:40AM +0530, Satyam Sharma wrote:
> > > Hmmm, actually those other users could easily write and maintain
> > > a 20-line patch that does the wait for async scans thing for them
> > > using /proc/scsi/scsi in any case.
> >
> > How about the three users who're bothered by this extra module being
> > built maintain a one-line patch to Kconfig and leave well enough alone?

So you expect users bothered with this to actually get on lkml / write to it
and complain about this? And because not everybody else who is
disgusted with this user-invisible-default-m-module-way-of-solving-this-problem
(when it shouldn't be a module at all) is doing that, it's just "the three"?

It is *shocking* / funny how you *still* want to defend that:

static int __init wait_scan_init(void)
{
scsi_complete_async_scans();
return 0;
/* BTW this could've been return scsi_complete_async_scans();
* I see scsi_complete_async_scans() never fails, but still. */
}
late_initcall(wait_scan_init);

deserves/must be a separate module, and that doing:

config SCSI_WAIT_SCAN
tristate
default m

is the best way to solve this !!!

In any case, firstly, I'm not a user of SCSI at all. I'm still
interested in this,
but because for me (like I've said twice already) this is simply a (trivial,
perhaps) matter of doing something in the kernel in a better/proper way,
than what is being done currently.

It's also somewhat a matter of *taste* (and hence subjective), if you
_still_ don't get it, Matthew, then there's no point continuing this thread
and trying to convince you ad infinitum.

On 5/18/07, Benjamin LaHaise <[email protected]> wrote:
> The module has an added bonus that it doesn't require any new tools to
> make work. Doing it via sysfs/procfs means a new rev of whatever tool
> generates the boot initrd, plus fixing up boot scripts. Loading a module
> can be done via a simple option to the existing boot tools.

I do not expect the alternative ways to change this that we've discussed
so far to necessitate any major "fixing up", but yeah a minor touch-up
would clearly be required.

2007-05-18 05:28:21

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On 5/18/07, Matthew Wilcox <[email protected]> wrote:
> On Thu, May 17, 2007 at 03:43:26PM -0400, Benjamin LaHaise wrote:
> > On Thu, May 17, 2007 at 01:39:54PM -0600, Matthew Wilcox wrote:
> > > On Fri, May 18, 2007 at 12:34:40AM +0530, Satyam Sharma wrote:
> > > > Hmmm, actually those other users could easily write and maintain
> > > > a 20-line patch that does the wait for async scans thing for them
> > > > using /proc/scsi/scsi in any case.
> > >
> > > How about the three users who're bothered by this extra module being
> > > built maintain a one-line patch to Kconfig and leave well enough alone?

[ BTW, this is the last time I'll try explaining this to you. ]

The one-line patch you're suggesting *would*not*allow* one to use the async
scanning _at_all_. If one really wants to use async scanning reliably (even in
the future, as it can be turned on at boot-time later, like you very well know),
that module *must* be built. Making it user-visible and/or optional would *not*
be a solution but a *problem*. What I have been suggesting is *not* to make
this *dummy module* user-visible and/or optional but to _not_ use this
*dummy module* for this purpose in the first place.

[ There. What was this ... third ... fourth time? ]

> > The module has an added bonus that it doesn't require any new tools to
> > make work. Doing it via sysfs/procfs means a new rev of whatever tool
> > generates the boot initrd, plus fixing up boot scripts. Loading a module
> > can be done via a simple option to the existing boot tools.
>
> That was what James and I thought. However, the distros seem unhappy
> with it. Of course, they won't actually *comment* on it, they just
> disable the async scan and won't talk about why.

[ This time, I don't see the subject changing, nor a "change in the general
direction of the thread blah blah blah", and still you feel compelled to not
maintain the CC list. Wow. ]

2007-05-18 11:19:19

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Fri, May 18, 2007 at 09:11:58AM +0530, Satyam Sharma wrote:
> It's also somewhat a matter of *taste* (and hence subjective), if you
> _still_ don't get it, Matthew, then there's no point continuing this thread
> and trying to convince you ad infinitum.

Right. It's a matter of taste. What makes you think you have taste?

I don't think that the module solution is perfect. But abusing the
module parameters is a worse idea. sysfs just isn't a good fit for this,
according to my taste.

2007-05-18 11:24:48

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On Fri, May 18, 2007 at 10:58:05AM +0530, Satyam Sharma wrote:
> [ BTW, this is the last time I'll try explaining this to you. ]

Oh good. Perhaps you can just drop the idea entirely and give up?

> The one-line patch you're suggesting *would*not*allow* one to use the async
> scanning _at_all_. If one really wants to use async scanning reliably (even
> in
> the future, as it can be turned on at boot-time later, like you very well
> know),
> that module *must* be built. Making it user-visible and/or optional would
> *not*
> be a solution but a *problem*. What I have been suggesting is *not* to make
> this *dummy module* user-visible and/or optional but to _not_ use this
> *dummy module* for this purpose in the first place.

That's simply not true. There are other ways of using async scanning
reliably -- as Peter Jones pointed out. If you're relying on the earlier
semantics of "modprobe returned, therefore scanning is complete", then
yes, it's unreliable. But if you're using kevents/udev/etc to find out
when devices have been discovered, then it's not unreliable.

> [ This time, I don't see the subject changing, nor a "change in the general
> direction of the thread blah blah blah", and still you feel compelled to not
> maintain the CC list. Wow. ]

I see trimming the CC list as a courtesy to those who've had enough of
this pointless thread landing in their mailboxes.

2007-05-18 13:06:58

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On 5/18/07, Matthew Wilcox <[email protected]> wrote:
> On Fri, May 18, 2007 at 09:11:58AM +0530, Satyam Sharma wrote:
> > It's also somewhat a matter of *taste* (and hence subjective), if you
> > _still_ don't get it, Matthew, then there's no point continuing this thread
> > and trying to convince you ad infinitum.
>
> Right. It's a matter of taste. What makes you think you have taste?

Well, my stand uptil now has been to consider as many options as
possible. I have certainly not married myself to just one particular way
of doing this -- but I bet that the one way that I do _dislike_, the dummy
module one, would not be found tasteful / best by most people around
(yourself included, as you say).

> I don't think that the module solution is perfect. But abusing the
> module parameters is a worse idea. sysfs just isn't a good fit for this,
> according to my taste.

The whole point is to at least _consider_ other alternatives. And I've
found your attitude so far to have been extremely blocking / difficult.

2007-05-18 13:14:38

by Satyam Sharma

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

On 5/18/07, Matthew Wilcox <[email protected]> wrote:
> On Fri, May 18, 2007 at 10:58:05AM +0530, Satyam Sharma wrote:
> > [ BTW, this is the last time I'll try explaining this to you. ]
>
> Oh good. Perhaps you can just drop the idea entirely and give up?

Well, I do plan to, at least as far as convincing you or getting some
alternative in mainline is concerned.

> > The one-line patch you're suggesting *would*not*allow* one to use the async
> > scanning _at_all_. If one really wants to use async scanning reliably (even
> > in
> > the future, as it can be turned on at boot-time later, like you very well
> > know),
> > that module *must* be built. Making it user-visible and/or optional would
> > *not*
> > be a solution but a *problem*. What I have been suggesting is *not* to make
> > this *dummy module* user-visible and/or optional but to _not_ use this
> > *dummy module* for this purpose in the first place.
>
> That's simply not true. There are other ways of using async scanning
> reliably -- as Peter Jones pointed out. If you're relying on the earlier
> semantics of "modprobe returned, therefore scanning is complete", then
> yes, it's unreliable. But if you're using kevents/udev/etc to find out
> when devices have been discovered, then it's not unreliable.

But I do hope the mainline kernel does want to give (by default) the best
way/interface of doing this for users/distros, at least. From Peter's mail,
I gather that we can do better than what we are presently doing.

> > [ This time, I don't see the subject changing, nor a "change in the general
> > direction of the thread blah blah blah", and still you feel compelled to not
> > maintain the CC list. Wow. ]
>
> I see trimming the CC list as a courtesy to those who've had enough of
> this pointless thread landing in their mailboxes.

Well you trimmed out those original "three users" who did show interest in
this in the first place. Anyway, pointless it indeed has become...

2007-05-18 14:00:35

by Stefan Richter

[permalink] [raw]
Subject: Re: Asynchronous scsi scanning

Peter Jones wrote:
> So really, either way means we need to update the tools. It also
> doesn't really solve the problem -- when I insert "usb-storage", the
> SCSI scan may still finish while we're still enumerating the bus for USB
> devices. (I'd be willing to believe I'm wrong about this specific
> example, but I suspect the principle still applies for some other driver.)
>
> In practice, we wind up doing the compare/timeout loop as on
> /proc/scsi/scsi, but on /proc/bus/usb/devices or
> /sys/bus/ieee1394/drivers/sbp2 instead.

sbp2 and its successor fw-sbp2 are not yet integrated with
scsi_wait_scan, but should be _some_ day.
http://bugzilla.kernel.org/show_bug.cgi?id=3225#c2
--
Stefan Richter
-=====-=-=== -=-= =--=-
http://arcgraph.de/sr/

2007-05-19 18:50:19

by Greg KH

[permalink] [raw]
Subject: Re: sysfs makes scaling suck Re: Asynchronous scsi scanning

On Thu, May 17, 2007 at 01:49:52PM -0400, Benjamin LaHaise wrote:
> On Thu, May 17, 2007 at 01:45:24PM -0400, James Bottomley wrote:
> > But also, the sysfs with over 4,000 (and higher) devices was
> > specifically checked by OSDL (actually as part of the CGL testing) some
> > of the Manoj changes (for unpinning entries etc) were needed to get it
> > to function, but as of now, I believe an enterprise scaling test works
> > reasonably well for it ... there certainly wasn't any evidence of it
> > dying horribly in the tests.
>
> i386 exhausts lowmem very quickly. SCSI is in a bit better shape than
> network devices as the multiplier is only around 4 compared to 16 for network
> devices.

And sysfs pushes nodes out of memory when we start to exhaust memory, so
there should not be a problem at all. If there is, please let the sysfs
developers know about it and we will work to fix this.

thanks,

greg k-h