[patch] x86_64: fix earlyprintk=...,keep regression

[patch] x86_64: fix earlyprintk=...,keep regression

[Ingo Molnar]

Subject: [patch] x86_64: fix earlyprintk=...,keep regression
From: Ingo Molnar <mingo@elte.hu>

the following cleanup patch:

 commit 2c8c0e6b8d7700a990da8d24eff767f9ca223b96
 Author: Andi Kleen <ak@suse.de>
 Date:   Tue Sep 26 10:52:32 2006 +0200

     [PATCH] Convert x86-64 to early param

     Instead of hackish manual parsing

broke the earlyprintk=...,keep feature. This patch restores that 
functionality. Tested on x86_64. Must-have for v2.6.19, no risk.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86_64/kernel/early_printk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/arch/x86_64/kernel/early_printk.c
===================================================================
--- linux.orig/arch/x86_64/kernel/early_printk.c
+++ linux/arch/x86_64/kernel/early_printk.c
@@ -224,7 +224,7 @@ static int __init setup_early_printk(cha
 		return 0;
 	early_console_initialized = 1;
 
-	if (!strcmp(buf,"keep"))
+	if (strstr(buf, "keep"))
 		keep_early = 1;
 
 	if (!strncmp(buf, "serial", 6)) {

Re: [patch] x86_64: fix earlyprintk=...,keep regression

[Linus Torvalds]

On Tue, 28 Nov 2006, Ingo Molnar wrote:
> 
> the following cleanup patch:
> 
>  commit 2c8c0e6b8d7700a990da8d24eff767f9ca223b96
>  Author: Andi Kleen <ak@suse.de>
>  Date:   Tue Sep 26 10:52:32 2006 +0200
> 
>      [PATCH] Convert x86-64 to early param
> 
>      Instead of hackish manual parsing
> 
> broke the earlyprintk=...,keep feature. This patch restores that 
> functionality. Tested on x86_64. Must-have for v2.6.19, no risk.

Hmm.

> -	if (!strcmp(buf,"keep"))
> +	if (strstr(buf, "keep"))
>  		keep_early = 1;
>  
>  	if (!strncmp(buf, "serial", 6)) {

This is pretty ugly. The whole reason for the bug in the first place was 
that "keep" was tested differently from all the other cases, and now you 
just perpetuate that.

All the other strings are tested with "!strncmp(..)", so I think this one 
should too. No?

Something like the appended (where I made the patch a bit bigger by just 
making it visually look the same too and adding the "} else if .." thing.

Or is there some reason you really _want_ "keep" to be different? If so, 
it should probably be commented on.

		Linus
---
diff --git a/arch/x86_64/kernel/early_printk.c b/arch/x86_64/kernel/early_printk.c
index e22ecd5..51e6912 100644
--- a/arch/x86_64/kernel/early_printk.c
+++ b/arch/x86_64/kernel/early_printk.c
@@ -224,10 +224,9 @@ static int __init setup_early_printk(char *buf)
 		return 0;
 	early_console_initialized = 1;
 
-	if (!strcmp(buf,"keep"))
+	if (!strncmp(buf,"keep", 4)) {
 		keep_early = 1;
-
-	if (!strncmp(buf, "serial", 6)) {
+	} else if (!strncmp(buf, "serial", 6)) {
 		early_serial_init(buf + 6);
 		early_console = &early_serial_console;
 	} else if (!strncmp(buf, "ttyS", 4)) {

Re: [patch] x86_64: fix earlyprintk=...,keep regression

[Linus Torvalds]

On Tue, 28 Nov 2006, Linus Torvalds wrote:
> 
> Or is there some reason you really _want_ "keep" to be different? If so, 
> it should probably be commented on.

Hmm. Looking at the commit that broke, the "strstr()" was there 
originally, so in that sense your patch is obviously the minimal and safe 
one. So I'll apply it as-is after all, but I'd be even happier if somebody 
cleaned this up a bit and sent a patch after I do 2.6.19 (which may well 
be later today, I think the time has come).

		Linus

Re: [patch] x86_64: fix earlyprintk=...,keep regression

[Andi Kleen]

> 
> Or is there some reason you really _want_ "keep" to be different? If so, 
> it should probably be commented on.

It's just that keep is the only option that can only be at the end, all
others can be followed by more, so it
made minor sense to use strcmp() to match the \0 too. But using strncmp
everywhere is fine too since the syntax checking on these things is always
quite weak and it probably doesn't make much difference either way. 

I would have gone with Ingo's fix, but if you prefer strncmp..? 

-Andi

Re: [patch] x86_64: fix earlyprintk=...,keep regression

[Ingo Molnar]

* Linus Torvalds <torvalds@osdl.org> wrote:

> -	if (!strcmp(buf,"keep"))
> +	if (!strncmp(buf,"keep", 4)) {
>  		keep_early = 1;
> -
> -	if (!strncmp(buf, "serial", 6)) {
> +	} else if (!strncmp(buf, "serial", 6)) {
>  		early_serial_init(buf + 6);
>  		early_console = &early_serial_console;

nope, that doesnt work, because the function call is a one-time thing 
via the early_console_initialized flag. Nor does this keep compatibility 
with the 2.6.18 API, my existing boot-entries:

        root (hd0,4)
        kernel /boot/bzImage-x64 root=/dev/hda5 \
	earlyprintk=serial,ttyS0,115200,keep console=ttyS0,115200 console=tty

stopped working.

I agree that the parameter parsing here is a bit hacky, but my patch 
restores the original behavior, so i think that's the best option for 
now.

	Ingo