public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RE: idebus setup problem (2.6.7-rc1)
@ 2004-06-07 11:04 Zhu, Yi
  2004-06-07 23:00 ` Rusty Russell
  0 siblings, 1 reply; 22+ messages in thread
From: Zhu, Yi @ 2004-06-07 11:04 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Bartlomiej Zolnierkiewicz, Herbert Poetzl, Auzanneau Gregory,
	Jeff Garzik, lkml - Kernel Mailing List, Andrew Morton

Rusty Russell wrote:
> 
> OK, I've revisited this problem, with my thinking cap ON this time.
> Sorry for the delay. 
> 
> Andrew, please revert kernel-parameter-parsing-fix.patch and
> kernel-parameter-parsing-fix-fix.patch in favor of this one-liner.
> 
> Yi, does this fix your ACPI problem?
> 
> Rusty.
> 
> Name: Handle __early_param and __setup Collision
> Status: Trivial
> 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".

Really? I think currently only ide_setup is an exception for __setup(),
which will match all params given in command line.

> 
> 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 .22424-linux-2.6.7-rc2-bk7/init/main.c
> .22424-linux-2.6.7-rc2-bk7.updated/init/main.c
> --- .22424-linux-2.6.7-rc2-bk7/init/main.c	2004-06-07
> 09:51:11.000000000 +1000 +++
> .22424-linux-2.6.7-rc2-bk7.updated/init/main.c 2004-06-07
> 09:53:06.000000000 +1000 @@ -159,8 +159,9 @@ 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)
> +			/* 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.

Thanks,
-yi

--- linux-2.6.7-rc2-mm2.orig/init/main.c	2004-06-07
16:01:25.000000000 +0800
+++ linux-2.6.7-rc2-mm2/init/main.c	2004-06-07 18:50:22.256204536
+0800
@@ -154,18 +154,22 @@ static int __init obsolete_checksetup(ch
 {
 	struct obs_kernel_param *p;
 	extern struct obs_kernel_param __setup_start, __setup_end;
-	char *ptr;
-	int len = strlen(line);
 
-	if ((ptr = strchr(line, '=')))
-		len = ptr - line;
 	p = &__setup_start;
 	do {
 		int n = strlen(p->str);
-		if (n == 0 || (len <= n && !strncmp(line, p->str, n))) {
-			/* Already done in parse_early_param? */
-			if (p->early)
-				return 1;
+		if (!strncmp(line, p->str, n)) {
+			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;
+				else {
+					p++;
+					continue;
+				}
+			}
 			if (!p->setup_func) {
 				printk(KERN_WARNING "Parameter %s is
obsolete,"
 						" ignored\n", p->str);


^ permalink raw reply	[flat|nested] 22+ messages in thread
* RE: idebus setup problem (2.6.7-rc1)
@ 2004-06-06 13:11 Zhu, Yi
  2004-06-07  0:07 ` Rusty Russell
  0 siblings, 1 reply; 22+ messages in thread
From: Zhu, Yi @ 2004-06-06 13:11 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Rusty Russell
  Cc: Herbert Poetzl, Auzanneau Gregory, Jeff Garzik,
	lkml - Kernel Mailing List, Andrew Morton

Bartlomiej Zolnierkiewicz wrote:
> 
> Why can't we apply this minimal fix from Yi for now?

Thanks, this is in 2.6.7-rc2-mm2.

> 
> --- linux-2.6.7-rc1-mm1.orig/init/main.c        2004-05-28
> 12:39:15.549314064 +0800 +++ linux-2.6.7-rc1-mm1/init/main.c    
> 2004-05-28 12:40:29.399087192 +0800
> @@ -162,7 +162,7 @@ static int __init obsolete_checksetup(ch        
>         p = &__setup_start; do {
>                 int n = strlen(p->str);
> -               if (len <= n && !strncmp(line, p->str, n)) {
> +               if (n == 0 || (len <= n && !strncmp(line, p->str,
>                         n))) { /* Already done in parse_early_param?
>                         */ if (p->early)
>                                 return 1;
> 

Thanks,
-yi

^ permalink raw reply	[flat|nested] 22+ messages in thread
* RE: idebus setup problem (2.6.7-rc1)
@ 2004-06-03 14:05 Zhu, Yi
  2004-06-03 21:31 ` Herbert Poetzl
  0 siblings, 1 reply; 22+ messages in thread
From: Zhu, Yi @ 2004-06-03 14:05 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Bartlomiej Zolnierkiewicz, Auzanneau Gregory, Jeff Garzik,
	lkml - Kernel Mailing List, Andrew Morton

Rusty Russell wrote:
> 
> Dislike this idea.  If you have hundreds of parameters, maybe it's
> supposed to be a PITA? 

What's your idea to make module_param support alterable param
names like ide3=xxx ?

Thanks,
-yi


^ permalink raw reply	[flat|nested] 22+ messages in thread
* RE: idebus setup problem (2.6.7-rc1)
@ 2004-05-27 16:38 Zhu, Yi
  2004-06-03  6:53 ` Rusty Russell
  0 siblings, 1 reply; 22+ messages in thread
From: Zhu, Yi @ 2004-05-27 16:38 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Auzanneau Gregory, Jeff Garzik
  Cc: linux-kernel, Andrew Morton, Rusty Russell

linux-kernel-owner@vger.kernel.org wrote:
> Bartlomiej Zolnierkiewicz wrote:
>> 
>> It breaks all "idex=" and "hdx=" options.
>> Please take a look at how ide_setup().
> 
> Yes, thanks for pointing out. Maybe we need some wildcard
> support. If module_param() can do this, that's great.

Does below change acceptable to make module_param support 
wildcard '?' ?

--- linux-2.6.6.orig/kernel/params.c        2004-05-28
00:13:24.931368296 +0800
+++ linux-2.6.6/kernel/params.c     2004-05-28 00:16:04.406124448 +0800
@@ -37,7 +37,8 @@ static inline int dash2underscore(char c
 static inline int parameq(const char *input, const char *paramname)
 {
        unsigned int i;
-       for (i = 0; dash2underscore(input[i]) == paramname[i]; i++)
+       for (i = 0; paramname[i] == '?' ||
+            dash2underscore(input[i]) == paramname[i]; i++)
                if (input[i] == '\0')
                        return 1;
        return 0;



^ permalink raw reply	[flat|nested] 22+ messages in thread
* RE: idebus setup problem (2.6.7-rc1)
@ 2004-05-27 16:16 Zhu, Yi
  0 siblings, 0 replies; 22+ messages in thread
From: Zhu, Yi @ 2004-05-27 16:16 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Auzanneau Gregory, Jeff Garzik
  Cc: linux-kernel, Andrew Morton

Bartlomiej Zolnierkiewicz wrote:
> 
> It breaks all "idex=" and "hdx=" options.
> Please take a look at how ide_setup().

Yes, thanks for pointing out. Maybe we need some
wildcard support. If module_param() can do this, that's
great.

-yi

^ permalink raw reply	[flat|nested] 22+ messages in thread
* RE: idebus setup problem (2.6.7-rc1)
@ 2004-05-27 15:21 Zhu, Yi
  2004-05-27 15:51 ` Bartlomiej Zolnierkiewicz
  2004-05-27 16:09 ` Jeff Garzik
  0 siblings, 2 replies; 22+ messages in thread
From: Zhu, Yi @ 2004-05-27 15:21 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, Auzanneau Gregory; +Cc: linux-kernel, Andrew Morton

Bartlomiej Zolnierkiewicz [B.Zolnierkiewicz@elka.pw.edu.pl] wrote:
> 
> I remember seeing patch related to handling '=' in kernel
> params, maybe it's related (or maybe not).

Yes, this is caused by my kernel-parameter-parsing-fix.patch.

But I think below code in ide.c is a hack.
__setup("", ide_setup);

How about below change?

--- linux-2.6.7-rc1-mm1.orig/drivers/ide/ide.c      2004-05-27
23:07:59.405138992 +0800
+++ linux-2.6.7-rc1-mm1/drivers/ide/ide.c   2004-05-27
23:09:47.529701560 +0800
@@ -2459,7 +2459,8 @@ void cleanup_module (void)

 #else /* !MODULE */

-__setup("", ide_setup);
+__setup("hd", ide_setup);
+__setup("ide", ide_setup);

 module_init(ide_init);


-yi

^ permalink raw reply	[flat|nested] 22+ messages in thread
* idebus setup problem (2.6.7-rc1)
@ 2004-05-27 11:57 Auzanneau Gregory
  2004-05-27 14:37 ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 22+ messages in thread
From: Auzanneau Gregory @ 2004-05-27 11:57 UTC (permalink / raw)
  To: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]


Hello,

It seems there is a problem with idebus parameter with 2.6.7-rc1.
Indeed, it doesn't take into account lilo append.

With 2.6.7-rc1-mm1, i've got:
Kernel command line: BOOT_IMAGE=LinuxNEW ro root=304 idebus=66
ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx

With 2.6.6-mm3, i've got:
May 24 11:37:43 greg-port kernel: Kernel command line:
BOOT_IMAGE=LinuxNEW ro root=304 idebus=66
May 24 11:37:43 greg-port kernel: ide_setup: idebus=66
May 24 11:37:43 greg-port kernel: ide: Assuming 66MHz system bus speed
for PIO modes

I tried to seek in the code, but my level is not as good as I would like
it. :)


Thank you all for the good work with linux, keep up with it ! :)

-- 
Auzanneau Grégory
GPG 0x99137BEE

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2004-06-07 23:01 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <3ACA40606221794F80A5670F0AF15F84047998B0@PDSMSX403.ccr.corp.intel.com>
2004-05-28  4:50 ` idebus setup problem (2.6.7-rc1) Zhu, Yi
2004-05-28  9:11   ` Auzanneau Gregory
2004-05-28 12:50     ` Bartlomiej Zolnierkiewicz
2004-05-28 12:52       ` Auzanneau Gregory
2004-05-28 13:10         ` Bartlomiej Zolnierkiewicz
2004-06-07 11:04 Zhu, Yi
2004-06-07 23:00 ` Rusty Russell
  -- strict thread matches above, loose matches on Subject: below --
2004-06-06 13:11 Zhu, Yi
2004-06-07  0:07 ` Rusty Russell
2004-06-03 14:05 Zhu, Yi
2004-06-03 21:31 ` Herbert Poetzl
2004-06-03 21:44   ` Bartlomiej Zolnierkiewicz
2004-06-04  4:32     ` Rusty Russell
2004-06-04 22:58       ` Bartlomiej Zolnierkiewicz
2004-05-27 16:38 Zhu, Yi
2004-06-03  6:53 ` Rusty Russell
2004-05-27 16:16 Zhu, Yi
2004-05-27 15:21 Zhu, Yi
2004-05-27 15:51 ` Bartlomiej Zolnierkiewicz
2004-05-27 16:09 ` Jeff Garzik
2004-05-27 11:57 Auzanneau Gregory
2004-05-27 14:37 ` Bartlomiej Zolnierkiewicz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox