RE: idebus setup problem (2.6.7-rc1)

[Rusty Russell <rusty@rustcorp.com.au>]

On Mon, 2004-06-07 at 21:04, Zhu, Yi wrote:
> > For simplicity, we overload the __setup section to contain
> > both __early_param and __setup, so we can check that all
> > options on the command line are taken by at least one of
> > them.  However, __early_param have different semantics the
> > __setup: in particular, __early_param("acpi"), must not match
> > anything but "acpi" and "acpi=", which mirrors
> > module_param(), whereas __setup("acpi") would match anything
> > which starts with "acpi".
> 
> Really? I think currently only ide_setup is an exception for __setup(),
> which will match all params given in command line.

No, there is at least one other place which relies on this feature last
I searched.  They're all supposed to be stem matches, so __setup("foo")
matches "foobar=100" for example.

> > -			/* Already done in parse_early_param? */
> > -			if (p->early)
> > +			/* Already done in parse_early_param?  (Needs
> > +			 * exact match on param part) */
> > +			if (p->early && (line[n] == '\0' ||
> > line[n] == '='))
> >  				return 1;
> >  			if (!p->setup_func) {
> >  				printk(KERN_WARNING "Parameter
> > %s is obsolete, ignored\n", p->str);
> 
> This doesn't work. The p->setup_func for "acpi" will still be called on
> behalf of "acpi_os_name".
> 
> Below change should work.

Good point, but I think this is clearer:

Name: Handle __early_param and __setup Collision
Status: Booted on 2.6.7-rc2-bk7
Depends: EarlyParam/early_param.patch.gz
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

Yi Zhu (yi.zhu@intel.com) points out the following problem:

In arch/i386/kernel/setup.c:
	__early_param("acpi", early_acpi);

In drivers/acpi/osl.c:
	__setup("acpi_os_name=", acpi_os_name_setup);

The problem command line looks like:

	"acpi=force acpi_os_name=my_override_name"

For simplicity, we overload the __setup section to contain both
__early_param and __setup, so we can check that all options on the
command line are taken by at least one of them.  However,
__early_param have different semantics the __setup: in particular,
__early_param("acpi"), must not match anything but "acpi" and "acpi=",
which mirrors module_param(), whereas __setup("acpi") would match
anything which starts with "acpi".

Fix the obsolete_checksetup code to take this difference into account
correctly.
 
diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .9590-linux-2.6.7-rc2-bk7/init/main.c .9590-linux-2.6.7-rc2-bk7.updated/init/main.c
--- .9590-linux-2.6.7-rc2-bk7/init/main.c	2004-06-08 08:44:23.000000000 +1000
+++ .9590-linux-2.6.7-rc2-bk7.updated/init/main.c	2004-06-08 08:48:15.000000000 +1000
@@ -159,10 +159,12 @@ static int __init obsolete_checksetup(ch
 	do {
 		int n = strlen(p->str);
 		if (!strncmp(line, p->str, n)) {
-			/* Already done in parse_early_param? */
-			if (p->early)
-				return 1;
-			if (!p->setup_func) {
+			if (p->early) {
+				/* Already done in parse_early_param?  (Needs
+				 * exact match on param part) */
+				if (line[n] == '\0' || line[n] == '=')
+					return 1;
+			} else if (!p->setup_func) {
 				printk(KERN_WARNING "Parameter %s is obsolete, ignored\n", p->str);
 				return 1;
 			} else if (p->setup_func(line + n))

-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell