[patch v2] Fixes and cleanups for earlyprintk aka boot console.

[patch v2] Fixes and cleanups for earlyprintk aka boot console.

[Gerd Hoffmann]

[-- Attachment #1: early-printk-boot-console.patch --]
[-- Type: text/plain, Size: 9877 bytes --]

The console subsystem already has an idea of a boot console, using the
CON_BOOT flag.  The implementation has some flaws though.  The major
problem is that presence of a boot console makes register_console()
ignore any other console devices (unless explicitly specified on the
kernel command line).

This patch fixes the console selection code to *not* consider a boot
console a full-featured one, so the first non-boot console registering
will become the default console instead.  This way the unregister call
for the boot console in the register_console() function actually
triggers and the handover from the boot console to the real console
device works smoothly.  Added a printk for the handover, so you know
which console device the output goes to when the boot console stops
printing messages.

The disable_early_printk() call is obsolete with that patch, explicitly
disabling the early console isn't needed any more as it works
automagically with that patch.

I've walked through the tree, dropped all disable_early_printk()
instances found below arch/ and tagged the consoles with CON_BOOT if
needed.  The code is tested on x86, sh (thanks to Paul) and mips
(thanks to Ralf).

Changes to last version: Rediffed against -rc3, adapted to mips
cleanups by Ralf, fixed "udbg-immortal" cmd line arg on powerpc.

Signed-off-by: Gerd Hoffmann <kraxel@exsuse.de>
Acked-by: Paul Mundt <lethal@linux-sh.org>
Acked-by: Ralf Baechle <ralf@linux-mips.org>
Cc: Andi Kleen <ak@suse.de>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
---
 arch/alpha/kernel/setup.c         |    9 ---------
 arch/alpha/kernel/srmcons.c       |    2 +-
 arch/mips/kernel/early_printk.c   |    5 -----
 arch/powerpc/kernel/udbg.c        |   19 ++++++-------------
 arch/sh/kernel/early_printk.c     |   18 ++++++------------
 arch/sh64/kernel/early_printk.c   |    8 +-------
 arch/x86_64/kernel/early_printk.c |   20 +++++---------------
 drivers/char/tty_io.c             |    5 -----
 kernel/printk.c                   |   26 ++++++++++++++++----------
 9 files changed, 35 insertions(+), 77 deletions(-)

===================================================================
Index: vanilla-2.6.21-rc3/arch/x86_64/kernel/early_printk.c
===================================================================
--- vanilla-2.6.21-rc3.orig/arch/x86_64/kernel/early_printk.c
+++ vanilla-2.6.21-rc3/arch/x86_64/kernel/early_printk.c
@@ -244,22 +244,12 @@ static int __init setup_early_printk(cha
  		early_console = &simnow_console;
  		keep_early = 1;
 	}
+
+	if (keep_early)
+		early_console->flags &= ~CON_BOOT;
+	else
+		early_console->flags |= CON_BOOT;
 	register_console(early_console);
 	return 0;
 }
-
 early_param("earlyprintk", setup_early_printk);
-
-void __init disable_early_printk(void)
-{
-	if (!early_console_initialized || !early_console)
-		return;
-	if (!keep_early) {
-		printk("disabling early console\n");
-		unregister_console(early_console);
-		early_console_initialized = 0;
-	} else {
-		printk("keeping early console\n");
-	}
-}
-
Index: vanilla-2.6.21-rc3/kernel/printk.c
===================================================================
--- vanilla-2.6.21-rc3.orig/kernel/printk.c
+++ vanilla-2.6.21-rc3/kernel/printk.c
@@ -931,8 +931,16 @@ void register_console(struct console *co
 {
 	int i;
 	unsigned long flags;
+	struct console *bootconsole = NULL;
 
-	if (preferred_console < 0)
+	if (console_drivers) {
+		if (console->flags & CON_BOOT)
+			return;
+		if (console_drivers->flags & CON_BOOT)
+			bootconsole = console_drivers;
+	}
+
+	if (preferred_console < 0 || bootconsole || !console_drivers)
 		preferred_console = selected_console;
 
 	/*
@@ -978,8 +986,11 @@ void register_console(struct console *co
 	if (!(console->flags & CON_ENABLED))
 		return;
 
-	if (console_drivers && (console_drivers->flags & CON_BOOT)) {
-		unregister_console(console_drivers);
+	if (bootconsole) {
+		printk(KERN_INFO "console handover: boot [%s%d] -> real [%s%d]\n",
+		       bootconsole->name, bootconsole->index,
+		       console->name, console->index);
+		unregister_console(bootconsole);
 		console->flags &= ~CON_PRINTBUFFER;
 	}
 
@@ -1030,16 +1041,11 @@ int unregister_console(struct console *c
 		}
 	}
 
-	/* If last console is removed, we re-enable picking the first
-	 * one that gets registered. Without that, pmac early boot console
-	 * would prevent fbcon from taking over.
-	 *
+	/*
 	 * If this isn't the last console and it has CON_CONSDEV set, we
 	 * need to set it on the next preferred console.
 	 */
-	if (console_drivers == NULL)
-		preferred_console = selected_console;
-	else if (console->flags & CON_CONSDEV)
+	if (console_drivers != NULL && console->flags & CON_CONSDEV)
 		console_drivers->flags |= CON_CONSDEV;
 
 	release_console_sem();
Index: vanilla-2.6.21-rc3/drivers/char/tty_io.c
===================================================================
--- vanilla-2.6.21-rc3.orig/drivers/char/tty_io.c
+++ vanilla-2.6.21-rc3/drivers/char/tty_io.c
@@ -141,8 +141,6 @@ static DECLARE_MUTEX(allocated_ptys_lock
 static int ptmx_open(struct inode *, struct file *);
 #endif
 
-extern void disable_early_printk(void);
-
 static void initialize_tty_struct(struct tty_struct *tty);
 
 static ssize_t tty_read(struct file *, char __user *, size_t, loff_t *);
@@ -3893,9 +3891,6 @@ void __init console_init(void)
 	 * set up the console device so that later boot sequences can 
 	 * inform about problems etc..
 	 */
-#ifdef CONFIG_EARLY_PRINTK
-	disable_early_printk();
-#endif
 	call = __con_initcall_start;
 	while (call < __con_initcall_end) {
 		(*call)();
Index: vanilla-2.6.21-rc3/arch/alpha/kernel/setup.c
===================================================================
--- vanilla-2.6.21-rc3.orig/arch/alpha/kernel/setup.c
+++ vanilla-2.6.21-rc3/arch/alpha/kernel/setup.c
@@ -744,15 +744,6 @@ setup_arch(char **cmdline_p)
 	paging_init();
 }
 
-void __init
-disable_early_printk(void)
-{
-	if (alpha_using_srm && srmcons_output) {
-		unregister_srm_console();
-		srmcons_output = 0;
-	}
-}
-
 static char sys_unknown[] = "Unknown";
 static char systype_names[][16] = {
 	"0",
Index: vanilla-2.6.21-rc3/arch/alpha/kernel/srmcons.c
===================================================================
--- vanilla-2.6.21-rc3.orig/arch/alpha/kernel/srmcons.c
+++ vanilla-2.6.21-rc3/arch/alpha/kernel/srmcons.c
@@ -300,7 +300,7 @@ static struct console srmcons = {
 	.write		= srm_console_write,
 	.device		= srm_console_device,
 	.setup		= srm_console_setup,
-	.flags		= CON_PRINTBUFFER,
+	.flags		= CON_PRINTBUFFER | CON_BOOT,
 	.index		= -1,
 };
 
Index: vanilla-2.6.21-rc3/arch/powerpc/kernel/udbg.c
===================================================================
--- vanilla-2.6.21-rc3.orig/arch/powerpc/kernel/udbg.c
+++ vanilla-2.6.21-rc3/arch/powerpc/kernel/udbg.c
@@ -142,29 +142,22 @@ static void udbg_console_write(struct co
 static struct console udbg_console = {
 	.name	= "udbg",
 	.write	= udbg_console_write,
-	.flags	= CON_PRINTBUFFER | CON_ENABLED,
+	.flags	= CON_PRINTBUFFER | CON_ENABLED | CON_BOOT,
 	.index	= -1,
 };
 
 static int early_console_initialized;
 
-void __init disable_early_printk(void)
-{
-	if (!early_console_initialized)
-		return;
-	if (strstr(boot_command_line, "udbg-immortal")) {
-		printk(KERN_INFO "early console immortal !\n");
-		return;
-	}
-	unregister_console(&udbg_console);
-	early_console_initialized = 0;
-}
-
 /* called by setup_system */
 void register_early_udbg_console(void)
 {
 	if (early_console_initialized)
 		return;
+
+	if (strstr(boot_command_line, "udbg-immortal")) {
+		printk(KERN_INFO "early console immortal !\n");
+		udbg_console.flags &= ~CON_BOOT;
+	}
 	early_console_initialized = 1;
 	register_console(&udbg_console);
 }
Index: vanilla-2.6.21-rc3/arch/sh/kernel/early_printk.c
===================================================================
--- vanilla-2.6.21-rc3.orig/arch/sh/kernel/early_printk.c
+++ vanilla-2.6.21-rc3/arch/sh/kernel/early_printk.c
@@ -192,20 +192,14 @@ int __init setup_early_printk(char *buf)
 	}
 #endif
 
-	if (likely(early_console))
+	if (likely(early_console)) {
+		if (keep_early)
+			early_console->flags &= ~CON_BOOT;
+		else
+			early_console->flags |= CON_BOOT;
 		register_console(early_console);
+	}
 
 	return 0;
 }
 early_param("earlyprintk", setup_early_printk);
-
-void __init disable_early_printk(void)
-{
-	if (!early_console_initialized || !early_console)
-		return;
-	if (!keep_early) {
-		printk("disabling early console\n");
-		unregister_console(early_console);
-	} else
-		printk("keeping early console\n");
-}
Index: vanilla-2.6.21-rc3/arch/sh64/kernel/early_printk.c
===================================================================
--- vanilla-2.6.21-rc3.orig/arch/sh64/kernel/early_printk.c
+++ vanilla-2.6.21-rc3/arch/sh64/kernel/early_printk.c
@@ -79,7 +79,7 @@ static struct console sh_console = {
 	.name		= "scifcon",
 	.write		= sh_console_write,
 	.setup		= sh_console_setup,
-	.flags		= CON_PRINTBUFFER,
+	.flags		= CON_PRINTBUFFER | CON_BOOT,
 	.index		= -1,
 };
 
@@ -97,9 +97,3 @@ void __init enable_early_printk(void)
 
 	register_console(&sh_console);
 }
-
-void disable_early_printk(void)
-{
-	unregister_console(&sh_console);
-}
-
Index: vanilla-2.6.21-rc3/arch/mips/kernel/early_printk.c
===================================================================
--- vanilla-2.6.21-rc3.orig/arch/mips/kernel/early_printk.c
+++ vanilla-2.6.21-rc3/arch/mips/kernel/early_printk.c
@@ -33,8 +33,3 @@ void __init setup_early_printk(void)
 {
 	register_console(&early_console);
 }
-
-void __init disable_early_printk(void)
-{
-	unregister_console(&early_console);
-}

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: [patch v2] Fixes and cleanups for earlyprintk aka boot console.

[John Stoffel]

Gerd> The console subsystem already has an idea of a boot console,
Gerd> using the CON_BOOT flag.  The implementation has some flaws
Gerd> though.  The major problem is that presence of a boot console
Gerd> makes register_console() ignore any other console devices
Gerd> (unless explicitly specified on the kernel command line).

Would this explain why recent version of GDM don't find the keyboard
properly when you boot with a kernel command line of:

   kernel ... console=tty0 console=ttyS1,96008N1

until you stop and restart GDM?  This was filed under Debian bug
#406457 (see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=406457)
and I could never figure it out until I removed my serial console line
at one point.  

I'll have to try this patch out and let you and the GDM 2.16.x
developers know what happens.

John

Re: [patch v2] Fixes and cleanups for earlyprintk aka boot console.

[Gerd Hoffmann]

John Stoffel wrote:
> Would this explain why recent version of GDM don't find the keyboard
> properly when you boot with a kernel command line of:
> 
>    kernel ... console=tty0 console=ttyS1,96008N1
> 
> until you stop and restart GDM?

No.  If you explicitly specify the console on the kernel command line
you get what you ask for, no matter whenever you apply the patch or not.

With the args above /dev/console is linked to /dev/ttyS1, which might
make gpm fail.  Would be a gpm bug though.

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

Re: [patch v2] Fixes and cleanups for earlyprintk aka boot console.

[Olaf Dabrunz]

On 15-Mar-07, John Stoffel wrote:
> Would this explain why recent version of GDM don't find the keyboard
> properly when you boot with a kernel command line of:
> 
>    kernel ... console=tty0 console=ttyS1,96008N1
> 
> until you stop and restart GDM?  This was filed under Debian bug
> #406457 (see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=406457)
> and I could never figure it out until I removed my serial console line
> at one point.  

Try changing the order of the console= parameters:

    kernel ... console=ttyS1,96008N1 console=tty0

Some time ago I understood what this does. Nowadays I can only remember
that this makes both the serial console and directly attached
screen/keyboard console work most of the time. FWIW, tty0 becomes the
so-called "preferred console".

-- 
Olaf Dabrunz (od/odabrunz), SUSE Linux Products GmbH, Nürnberg

Re: [patch v2] Fixes and cleanups for earlyprintk aka boot console.

[John Stoffel]

Sorry, I've been away for the past 10+ days and not reading email at
all.  

Olaf> On 15-Mar-07, John Stoffel wrote:
>> Would this explain why recent version of GDM don't find the keyboard
>> properly when you boot with a kernel command line of:
>> 
>> kernel ... console=tty0 console=ttyS1,96008N1
>> 
>> until you stop and restart GDM?  This was filed under Debian bug
>> #406457 (see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=406457)
>> and I could never figure it out until I removed my serial console line
>> at one point.  

Olaf> Try changing the order of the console= parameters:

Olaf>     kernel ... console=ttyS1,96008N1 console=tty0

This isn't good for me, since it then drops some output I want to save
via the serial console to the regular console.  It would be nice if
all output went to BOTH consoles, though input should only be accepted
on the first console listed.  

Olaf> Some time ago I understood what this does. Nowadays I can only
Olaf> remember that this makes both the serial console and directly
Olaf> attached screen/keyboard console work most of the time. FWIW,
Olaf> tty0 becomes the so-called "preferred console".

Sure, but I want the serial console to be the preferred console for
output, so I can capture it all.  But this is still a good idea and
I'll test it out and see if GDM still has a problem with it like
that.  I suspect not, but we'll see.

THanks for the suggestion,
John

Re: [patch v2] Fixes and cleanups for earlyprintk aka boot console.

[Olaf Dabrunz]

On 03-Apr-07, John Stoffel wrote:
> 
> Sorry, I've been away for the past 10+ days and not reading email at
> all.  

NP. Happens to me too (as you can see).

> Olaf> Try changing the order of the console= parameters:
> 
> Olaf>     kernel ... console=ttyS1,96008N1 console=tty0
> 
> This isn't good for me, since it then drops some output I want to save
> via the serial console to the regular console.  It would be nice if
> all output went to BOTH consoles, though input should only be accepted
> on the first console listed.  

That is actually the underlying problem: the Linux kernel does not
provide the means to duplicate data streams in the kernel to several
devices. This has to be done in user-space. The main reason for this is
that error handling, locking and such things would need to be different
if streams had a fan-in or a fan-out. It tends to make things more
complicated.

But probably it is an interesting project to explore and define the
sematics of devices with fan-in and fan-out.

I can image that user-space programs would be able to add fan-in or
fan-out file-descriptors to an opened file-descriptor by using an
ioctl(), but then they would need to be prepared to deal with different
error handling and behaviour of such a "file".

Some devices (as the console device) could then have a pre-defined
fan-in or fan-out set by default, if opened with a special flag. A
program using this flag would then say: I am prepared to deal with
fan-in / fan-out semantics.

-- 
Olaf Dabrunz (od/odabrunz), SUSE Linux Products GmbH, Nürnberg

Re: [patch v2] Fixes and cleanups for earlyprintk aka boot console.

[Andrew Morton]

On Thu, 15 Mar 2007 16:46:39 +0100 Gerd Hoffmann <kraxel@suse.de> wrote:

> The console subsystem already has an idea of a boot console, using the
> CON_BOOT flag.  The implementation has some flaws though.  The major
> problem is that presence of a boot console makes register_console()
> ignore any other console devices (unless explicitly specified on the
> kernel command line).
> 
> This patch fixes the console selection code to *not* consider a boot
> console a full-featured one, so the first non-boot console registering
> will become the default console instead.  This way the unregister call
> for the boot console in the register_console() function actually
> triggers and the handover from the boot console to the real console
> device works smoothly.  Added a printk for the handover, so you know
> which console device the output goes to when the boot console stops
> printing messages.
> 
> The disable_early_printk() call is obsolete with that patch, explicitly
> disabling the early console isn't needed any more as it works
> automagically with that patch.
> 
> I've walked through the tree, dropped all disable_early_printk()
> instances found below arch/ and tagged the consoles with CON_BOOT if
> needed.  The code is tested on x86, sh (thanks to Paul) and mips
> (thanks to Ralf).
> 
> Changes to last version: Rediffed against -rc3, adapted to mips
> cleanups by Ralf, fixed "udbg-immortal" cmd line arg on powerpc.

I get this, across netconsole:

[17179569.184000] console handover: boot [earlyvga_f\x11_0] -> real [tty0]

wanna take a look at why there's cruft in bootconsole->name please?

in grub.conf I have

        kernel /boot/bzImage-2.6.21-rc7-mm1 ro root=LABEL=/ rhgb vga=0x263 netconsole=4444@192.168.2.10/eth0,5145@192.168.2.33/00:0D:56:C6:C6:CC profile=1 earlyprintk=vga resume=8:5 time

and I'm using

http://userweb.kernel.org/~akpm/config-sony.txt

Thanks.

Re: [patch v2] Fixes and cleanups for earlyprintk aka boot console.

[Gerd Hoffmann]

> I get this, across netconsole:
> 
> [17179569.184000] console handover: boot [earlyvga_f\x11_0] -> real [tty0]
> 
> wanna take a look at why there's cruft in bootconsole->name please?

-EFULL ;)

"earlyvga" is 8 chars.  struct console->name is char[8].  No space left 
for the trailing ´\0´, the cruft comes from the next field (write 
function pointer).  Obviously nobody ever printed the early console 
names before.

Hmm.  We can make the names shorter.  We can make the name field longer 
(probably 16, it ends up taking that much anyway due to aligments at 
least on 64bit).  This looks best to me.  We could also use 
printk("%.8s",name) to make printk stop after 8 chars, but I somehow 
don't like hardcoding the length like this ...

cheers,
   Gerd

Re: [patch v2] Fixes and cleanups for earlyprintk aka boot console.

[Andrew Morton]

On Tue, 24 Apr 2007 16:04:16 +0200 Gerd Hoffmann <kraxel@redhat.com> wrote:

> > I get this, across netconsole:
> > 
> > [17179569.184000] console handover: boot [earlyvga_f\x11_0] -> real [tty0]
> > 
> > wanna take a look at why there's cruft in bootconsole->name please?
> 
> -EFULL ;)
> 
> "earlyvga" is 8 chars.  struct console->name is char[8].  No space left 
> for the trailing ´\0´, the cruft comes from the next field (write 
> function pointer).  Obviously nobody ever printed the early console 
> names before.

doh.

> Hmm.  We can make the names shorter.  We can make the name field longer 
> (probably 16, it ends up taking that much anyway due to aligments at 
> least on 64bit).  This looks best to me.  We could also use 
> printk("%.8s",name) to make printk stop after 8 chars, but I somehow 
> don't like hardcoding the length like this ...
> 

yup, making it 16 sounds simplest.  I'll do the patch, thanks.