Today, register_console() assumes the following usage (with respect to boot
consoles/early_printk).
The first console to register with register_console() with a flags set to
CON_BOOT is the one and only bootconsole.
If another register_console() is called with an additional CON_BOOT, today it
is silently rejected.
As soon as a console without the CON_BOOT set calls register_console(), the
one and only bootconsole is automatically unregistered.
Once there is a "standard" console - register_console() will silently reject
any consoles with it's CON_BOOT, set.
This changeset allows multiple boot consoles, and changes the functionality
to, be mostly the same as the above.
Any number of bootconsoles can be registered.
A "real" console will unregister all the bootconsoles.
Once a "real" console is registered, no more bootconsoles can be added.
Thoughts?
The use case is to have a console buffer which goes to serial, and a console
buffer which goes to a fixed memory buffer at the same time. (serial is what
most people use, but if serial is hosed for some reason, having things in
a buffer (which gets printed out by the bootloader) is the only way to
tell what is going on).
If you don't object I will send a properly formatted patch...
-------------
Index: kernel/printk.c
===================================================================
--- kernel/printk.c (revision 6860)
+++ kernel/printk.c (working copy)
@@ -1126,9 +1126,24 @@
unsigned long flags;
struct console *bootconsole = NULL;
+ /* before we register a new CON_BOOT console, make sure we don't
+ * already have a valid console
+ */
if (console_drivers) {
- if (console->flags & CON_BOOT)
- return;
+ if (console->flags & CON_BOOT) {
+ for (bootconsole = console_drivers;
+ bootconsole != NULL;
+ bootconsole = bootconsole->next) {
+ if (!(bootconsole->flags & CON_BOOT))
+ break;
+ }
+ if ((console->flags & CON_BOOT) &&
+ !(bootconsole->flags & CON_BOOT))
+ return;
+ if (!(bootconsole->flags & CON_BOOT))
+ bootconsole = NULL;
+ }
+
if (console_drivers->flags & CON_BOOT)
bootconsole = console_drivers;
}
@@ -1195,15 +1210,32 @@
if (!(console->flags & CON_ENABLED))
return;
- if (bootconsole && (console->flags & CON_CONSDEV)) {
- printk(KERN_INFO "console handover: boot [%s%d] -> real [%s%d]\n",
- bootconsole->name, bootconsole->index,
- console->name, console->index);
- unregister_console(bootconsole);
+ if (bootconsole && (console->flags & CON_CONSDEV) &&
+ !(console->flags & CON_BOOT)) {
+ /* we need to iterate through twice, to make sure we print
+ * everything out, before we unregister the console(s)
+ */
+ printk(KERN_INFO "console handover: ");
+ for (bootconsole = console_drivers; bootconsole != NULL;
+ bootconsole = bootconsole->next) {
+ if (bootconsole->flags & CON_BOOT)
+ printk("boot [%s%d] ", bootconsole->name,
+ bootconsole->index);
+ }
+ printk(" -> real [%s%d]\n", console->name, console->index);
+ for (bootconsole = console_drivers; bootconsole != NULL;
+ bootconsole = bootconsole->next) {
+ if (bootconsole->flags & CON_BOOT)
+ unregister_console(bootconsole);
+ }
console->flags &= ~CON_PRINTBUFFER;
} else {
- printk(KERN_INFO "console [%s%d] enabled\n",
- console->name, console->index);
+ if (console->flags & CON_BOOT)
+ printk(KERN_INFO "bootconsole [%s%d] enabled\n",
+ console->name, console->index);
+ else
+ printk(KERN_INFO "console [%s%d] enabled\n",
+ console->name, console->index);
}
/*
* Robin Getz <[email protected]> wrote:
> Today, register_console() assumes the following usage (with
> respect to boot consoles/early_printk).
>
> The first console to register with register_console() with a flags
> set to CON_BOOT is the one and only bootconsole.
>
> If another register_console() is called with an additional
> CON_BOOT, today it is silently rejected.
>
> As soon as a console without the CON_BOOT set calls
> register_console(), the one and only bootconsole is automatically
> unregistered.
>
> Once there is a "standard" console - register_console() will
> silently reject any consoles with it's CON_BOOT, set.
>
>
> This changeset allows multiple boot consoles, and changes the
> functionality to, be mostly the same as the above.
>
> Any number of bootconsoles can be registered. A "real" console
> will unregister all the bootconsoles. Once a "real" console is
> registered, no more bootconsoles can be added.
>
> Thoughts?
>
> The use case is to have a console buffer which goes to serial, and
> a console buffer which goes to a fixed memory buffer at the same
> time. (serial is what most people use, but if serial is hosed for
> some reason, having things in a buffer (which gets printed out by
> the bootloader) is the only way to tell what is going on).
Looks genuinely useful ...
> If you don't object I will send a properly formatted patch...
Find some minor comments below:
> -------------
>
>
> Index: kernel/printk.c
> ===================================================================
> --- kernel/printk.c (revision 6860)
> +++ kernel/printk.c (working copy)
> @@ -1126,9 +1126,24 @@
> unsigned long flags;
> struct console *bootconsole = NULL;
>
> + /* before we register a new CON_BOOT console, make sure we don't
> + * already have a valid console
> + */
please use the customary comment style:
/*
* Comment .....
* ...... goes here:
*/
specified in Documentation/CodingStyle.
> if (console_drivers) {
> - if (console->flags & CON_BOOT)
> - return;
> + if (console->flags & CON_BOOT) {
> + for (bootconsole = console_drivers;
> + bootconsole != NULL;
> + bootconsole = bootconsole->next) {
> + if (!(bootconsole->flags & CON_BOOT))
> + break;
> + }
> + if ((console->flags & CON_BOOT) &&
> + !(bootconsole->flags & CON_BOOT))
> + return;
> + if (!(bootconsole->flags & CON_BOOT))
> + bootconsole = NULL;
> + }
This is really ugly to read mainly because the meat of the function,
the loop over all console drivers, is two indentation levels to the
right which creates line break artifacts. I'd suggest to put this
portion into a helper inline.
Also, the name 'bootconsole' is way too long for an iterator.
Something like 'con' would be more than enough. (and the new console
that is being registered could be new_con or so, to bring it in line
with this naming.)
Also, the new semantics here seem overly complex. Why not have a
state variable that tells us whether we are in the early boot phase
or not and warn about early consoles that get registered too late
and real consoles that get registered too early?
That way the CON_BOOT flag is really reduntant in terms of
semantics: all consoles registered before console_init() are early
ones, and all consoles registered after that are final consoles. (or
something like that)
Is there any reason why we'd want to do something more complex than
such simple rules?
> +
> if (console_drivers->flags & CON_BOOT)
> bootconsole = console_drivers;
> }
> @@ -1195,15 +1210,32 @@
> if (!(console->flags & CON_ENABLED))
> return;
>
> - if (bootconsole && (console->flags & CON_CONSDEV)) {
> - printk(KERN_INFO "console handover: boot [%s%d] -> real [%s%d]\n",
> - bootconsole->name, bootconsole->index,
> - console->name, console->index);
> - unregister_console(bootconsole);
> + if (bootconsole && (console->flags & CON_CONSDEV) &&
> + !(console->flags & CON_BOOT)) {
> + /* we need to iterate through twice, to make sure we print
> + * everything out, before we unregister the console(s)
> + */
(please fix the comment.)
> + printk(KERN_INFO "console handover: ");
> + for (bootconsole = console_drivers; bootconsole != NULL;
> + bootconsole = bootconsole->next) {
> + if (bootconsole->flags & CON_BOOT)
> + printk("boot [%s%d] ", bootconsole->name,
> + bootconsole->index);
> + }
> + printk(" -> real [%s%d]\n", console->name, console->index);
> + for (bootconsole = console_drivers; bootconsole != NULL;
> + bootconsole = bootconsole->next) {
> + if (bootconsole->flags & CON_BOOT)
> + unregister_console(bootconsole);
> + }
this too should probably move into a helper inline.
> console->flags &= ~CON_PRINTBUFFER;
> } else {
> - printk(KERN_INFO "console [%s%d] enabled\n",
> - console->name, console->index);
> + if (console->flags & CON_BOOT)
> + printk(KERN_INFO "bootconsole [%s%d] enabled\n",
> + console->name, console->index);
> + else
> + printk(KERN_INFO "console [%s%d] enabled\n",
> + console->name, console->index);
multi-line branch statemens should preferably come with curly
braces. And this could probably be written simpler as well, as:
printk(KERN_INFO "%sconsole [%s%d] enabled\n",
(con->flags & CON_BOOT) ? "boot" : "",
con->name, con->index);
Ingo
On Tue 30 Jun 2009 18:25, Ingo Molnar pondered:
>
> * Robin Getz <[email protected]> wrote:
>
> > Today, register_console() assumes the following usage (with
> > respect to boot consoles/early_printk).
> >
> > The first console to register with register_console() with a flags
> > set to CON_BOOT is the one and only bootconsole.
> >
> > If another register_console() is called with an additional
> > CON_BOOT, today it is silently rejected.
> >
> > As soon as a console without the CON_BOOT set calls
> > register_console(), the one and only bootconsole is automatically
> > unregistered.
> >
> > Once there is a "standard" console - register_console() will
> > silently reject any consoles with it's CON_BOOT, set.
> >
> >
> > This changeset allows multiple boot consoles, and changes the
> > functionality to, be mostly the same as the above.
> >
> > Any number of bootconsoles can be registered. A "real" console
> > will unregister all the bootconsoles. Once a "real" console is
> > registered, no more bootconsoles can be added.
> >
> > Thoughts?
> >
> > The use case is to have a console buffer which goes to serial, and
> > a console buffer which goes to a fixed memory buffer at the same
> > time. (serial is what most people use, but if serial is hosed for
> > some reason, having things in a buffer (which gets printed out by
> > the bootloader) is the only way to tell what is going on).
>
> Looks genuinely useful ...
Yeah, I found it so.
> > If you don't object I will send a properly formatted patch...
>
> Find some minor comments below:
>
> > -------------
> >
> >
> > Index: kernel/printk.c
> > ===================================================================
> > --- kernel/printk.c (revision 6860)
> > +++ kernel/printk.c (working copy)
> > @@ -1126,9 +1126,24 @@
> > unsigned long flags;
> > struct console *bootconsole = NULL;
> >
> > + /* before we register a new CON_BOOT console, make sure we don't
> > + * already have a valid console
> > + */
>
> please use the customary comment style:
>
> /*
> * Comment .....
> * ...... goes here:
> */
>
> specified in Documentation/CodingStyle.
OK.
> > if (console_drivers) {
> > - if (console->flags & CON_BOOT)
> > - return;
> > + if (console->flags & CON_BOOT) {
> > + for (bootconsole = console_drivers;
> > + bootconsole != NULL;
> > + bootconsole = bootconsole->next) {
> > + if (!(bootconsole->flags & CON_BOOT))
> > + break;
> > + }
> > + if ((console->flags & CON_BOOT) &&
> > + !(bootconsole->flags & CON_BOOT))
> > + return;
> > + if (!(bootconsole->flags & CON_BOOT))
> > + bootconsole = NULL;
> > + }
>
> This is really ugly to read mainly because the meat of the function,
> the loop over all console drivers, is two indentation levels to the
> right which creates line break artifacts. I'd suggest to put this
> portion into a helper inline.
>
> Also, the name 'bootconsole' is way too long for an iterator.
> Something like 'con' would be more than enough. (and the new console
> that is being registered could be new_con or so, to bring it in line
> with this naming.)
I was just re-using what was already there. If you don't mind reviewing a
larger patch - I'll change it to something shorter.
> Also, the new semantics here seem overly complex.
They are - I fixed things a little, but not as much as you suggest.
> Why not have a
> state variable that tells us whether we are in the early boot phase
> or not and warn about early consoles that get registered too late
> and real consoles that get registered too early?
That makes sense to me. Today - there are some bootconsoles (x86 and sh) that
accept a "keep" - still register early - but don't set the CON_BOOT, so they
get treated like a normal console (but are hooked up before console_init()).
This would not allow that to happen.... - is that really desired?
> That way the CON_BOOT flag is really reduntant in terms of
> semantics: all consoles registered before console_init() are early
> ones, and all consoles registered after that are final consoles. (or
> something like that)
Again - just using what is already there...
> Is there any reason why we'd want to do something more complex than
> such simple rules?
>
> > +
> > if (console_drivers->flags & CON_BOOT)
> > bootconsole = console_drivers;
> > }
> > @@ -1195,15 +1210,32 @@
> > if (!(console->flags & CON_ENABLED))
> > return;
> >
> > - if (bootconsole && (console->flags & CON_CONSDEV)) {
> > - printk(KERN_INFO "console handover: boot [%s%d] -> real
> [%s%d]\n",
> > - bootconsole->name, bootconsole->index,
> > - console->name, console->index);
> > - unregister_console(bootconsole);
> > + if (bootconsole && (console->flags & CON_CONSDEV) &&
> > + !(console->flags & CON_BOOT)) {
> > + /* we need to iterate through twice, to make sure we
> print
> > + * everything out, before we unregister the console(s)
> > + */
>
> (please fix the comment.)
will do.
> > + printk(KERN_INFO "console handover: ");
> > + for (bootconsole = console_drivers; bootconsole != NULL;
> > + bootconsole = bootconsole->next) {
> > + if (bootconsole->flags & CON_BOOT)
> > + printk("boot [%s%d] ",
> bootconsole->name,
> > + bootconsole->index);
> > + }
> > + printk(" -> real [%s%d]\n", console->name,
> console->index);
> > + for (bootconsole = console_drivers; bootconsole != NULL;
> > + bootconsole = bootconsole->next) {
> > + if (bootconsole->flags & CON_BOOT)
> > + unregister_console(bootconsole);
> > + }
>
> this too should probably move into a helper inline.
OK
> > console->flags &= ~CON_PRINTBUFFER;
> > } else {
> > - printk(KERN_INFO "console [%s%d] enabled\n",
> > - console->name, console->index);
> > + if (console->flags & CON_BOOT)
> > + printk(KERN_INFO "bootconsole [%s%d] enabled\n",
> > + console->name, console->index);
> > + else
> > + printk(KERN_INFO "console [%s%d] enabled\n",
> > + console->name, console->index);
>
> multi-line branch statemens should preferably come with curly
> braces. And this could probably be written simpler as well, as:
>
> printk(KERN_INFO "%sconsole [%s%d] enabled\n",
> (con->flags & CON_BOOT) ? "boot" : "",
> con->name, con->index);
OK.
* Robin Getz <[email protected]> wrote:
> On Tue 30 Jun 2009 18:25, Ingo Molnar pondered:
> >
> > * Robin Getz <[email protected]> wrote:
> >
> > > Today, register_console() assumes the following usage (with
> > > respect to boot consoles/early_printk).
> > >
> > > The first console to register with register_console() with a flags
> > > set to CON_BOOT is the one and only bootconsole.
> > >
> > > If another register_console() is called with an additional
> > > CON_BOOT, today it is silently rejected.
> > >
> > > As soon as a console without the CON_BOOT set calls
> > > register_console(), the one and only bootconsole is automatically
> > > unregistered.
> > >
> > > Once there is a "standard" console - register_console() will
> > > silently reject any consoles with it's CON_BOOT, set.
> > >
> > >
> > > This changeset allows multiple boot consoles, and changes the
> > > functionality to, be mostly the same as the above.
> > >
> > > Any number of bootconsoles can be registered. A "real" console
> > > will unregister all the bootconsoles. Once a "real" console is
> > > registered, no more bootconsoles can be added.
> > >
> > > Thoughts?
> > >
> > > The use case is to have a console buffer which goes to serial, and
> > > a console buffer which goes to a fixed memory buffer at the same
> > > time. (serial is what most people use, but if serial is hosed for
> > > some reason, having things in a buffer (which gets printed out by
> > > the bootloader) is the only way to tell what is going on).
> >
> > Looks genuinely useful ...
>
> Yeah, I found it so.
>
> > > If you don't object I will send a properly formatted patch...
> >
> > Find some minor comments below:
> >
> > > -------------
> > >
> > >
> > > Index: kernel/printk.c
> > > ===================================================================
> > > --- kernel/printk.c (revision 6860)
> > > +++ kernel/printk.c (working copy)
> > > @@ -1126,9 +1126,24 @@
> > > unsigned long flags;
> > > struct console *bootconsole = NULL;
> > >
> > > + /* before we register a new CON_BOOT console, make sure we don't
> > > + * already have a valid console
> > > + */
> >
> > please use the customary comment style:
> >
> > /*
> > * Comment .....
> > * ...... goes here:
> > */
> >
> > specified in Documentation/CodingStyle.
>
> OK.
>
> > > if (console_drivers) {
> > > - if (console->flags & CON_BOOT)
> > > - return;
> > > + if (console->flags & CON_BOOT) {
> > > + for (bootconsole = console_drivers;
> > > + bootconsole != NULL;
> > > + bootconsole = bootconsole->next) {
> > > + if (!(bootconsole->flags & CON_BOOT))
> > > + break;
> > > + }
> > > + if ((console->flags & CON_BOOT) &&
> > > + !(bootconsole->flags & CON_BOOT))
> > > + return;
> > > + if (!(bootconsole->flags & CON_BOOT))
> > > + bootconsole = NULL;
> > > + }
> >
> > This is really ugly to read mainly because the meat of the
> > function, the loop over all console drivers, is two indentation
> > levels to the right which creates line break artifacts. I'd
> > suggest to put this portion into a helper inline.
> >
> > Also, the name 'bootconsole' is way too long for an iterator.
> > Something like 'con' would be more than enough. (and the new
> > console that is being registered could be new_con or so, to
> > bring it in line with this naming.)
>
> I was just re-using what was already there. If you don't mind
> reviewing a larger patch - I'll change it to something shorter.
I dont mind it at all. We try to goad people into improving the
immediate context of code they touch. For the sake of network
effects and the viral spreading of good code and stuff.
(Assuming my suggestions are good that is.)
> > Also, the new semantics here seem overly complex.
>
> They are - I fixed things a little, but not as much as you
> suggest.
>
> > Why not have a state variable that tells us whether we are in
> > the early boot phase or not and warn about early consoles that
> > get registered too late and real consoles that get registered
> > too early?
>
> That makes sense to me. Today - there are some bootconsoles (x86
> and sh) that accept a "keep" - still register early - but don't
> set the CON_BOOT, so they get treated like a normal console (but
> are hooked up before console_init()).
>
> This would not allow that to happen.... - is that really desired?
Hm, i actually rely on 'earlyprintk=...,keep' myself sometimes.
I should really have noticed that ;-)
'keep' is really useful for some of the nastiest of crashes: where
we crash so hard and so fast that regular printk has no chance/time
to print something useful. On more than one occasion i got the
un-fancy early-printk stuff give me a vital clue before the kernel
crapped up - while normal printk wouldnt.
So you are right - we need an iteration over early consoles and
shuffle the keep-ones into the real console list, right?
Ingo
On Tue 30 Jun 2009 19:14, Ingo Molnar pondered:
> > > Why not have a state variable that tells us whether we are in
> > > the early boot phase or not and warn about early consoles that
> > > get registered too late and real consoles that get registered
> > > too early?
> >
> > That makes sense to me. Today - there are some bootconsoles (x86
> > and sh) that accept a "keep" - still register early - but don't
> > set the CON_BOOT, so they get treated like a normal console (but
> > are hooked up before console_init()).
> >
> > This would not allow that to happen.... - is that really desired?
>
> Hm, i actually rely on 'earlyprintk=...,keep' myself sometimes.
>
> I should really have noticed that ;-)
>
> 'keep' is really useful for some of the nastiest of crashes: where
> we crash so hard and so fast that regular printk has no chance/time
> to print something useful. On more than one occasion i got the
> un-fancy early-printk stuff give me a vital clue before the kernel
> crapped up - while normal printk wouldnt.
>
> So you are right - we need an iteration over early consoles and
> shuffle the keep-ones into the real console list, right?
My limited understanding is that the only difference between a "standard"
console, and a bootconsole is the flag CON_BOOT.
boot consoles get added as normal (in register_console()), then removed from
the console list (in unregister_console()) already - don't they?
The other thing I forgot to do was update disable_boot_consoles() - that is
fixed now too.
-Robin
From: Robin Getz <[email protected]>
Today, register_console() assumes the following usage:
- The first console to register with a flag set to CON_BOOT is the
one and only bootconsole.
- If another register_console() is called with an additional CON_BOOT,
it is silently rejected.
- As soon as a console without the CON_BOOT set calls registers
the bootconsole is automatically unregistered.
- Once there is a "real" console - register_console() will silently
reject any consoles with it's CON_BOOT flag set.
In many systems (alpha, blackfin, microblaze, mips, powerpc, sh, & x86), there
are early_printk implementations, which use the CON_BOOT which come out
serial ports, vga, usb, & memory buffers. In many embedded systems, it would
be nice to have two - in case the primary fails, you always have access to a
backup memory buffer - but this requires at least two CON_BOOT consoles.
This changeset allows multiple boot consoles, and changes the functionality
to, be mostly the same as the above.
- Any number CON_BOOT consoles of can be registered
- A "real" console will unregister all the CON_BOOT consoles
- Once a "real" console is registered, no more CON_BOOT consoles
can be added (still silently rejected)
Signed-off-by : Robin Getz <[email protected]>
---
With the changeset, on boot you get:
root:/> dmesg | grep console
bootconsole [early_shadow0] enabled
bootconsole [early_BFuart0] enabled
Kernel command line: root=/dev/mtdblock0 rw earlyprintk=serial,uart0,57600 earlyprintk=shadow console=ttyBF0,57600
console handover:boot [early_BFuart0] boot [early_shadow0] -> real [ttyBF0]
or
root:/> dmesg | grep console
Kernel command line: root=/dev/mtdblock0 rw console=ttyBF0,57600
console [ttyBF0] enabled
---
printk.c | 127 +++++++++++++++++++++++++++++++----------------------
1 file changed, 75 insertions(+), 52 deletions(-)
diff --git a/kernel/printk.c b/kernel/printk.c
index b4d97b5..d03322a 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -37,6 +37,12 @@
#include <asm/uaccess.h>
/*
+ * for_each_console() allows you to iterate on each console
+ */
+#define for_each_console(con) \
+ for (con = console_drivers; con != NULL; con = con->next)
+
+/*
* Architectures can override it:
*/
void asmlinkage __attribute__((weak)) early_printk(const char *fmt, ...)
@@ -412,7 +418,7 @@ static void __call_console_drivers(unsigned start, unsigned end)
{
struct console *con;
- for (con = console_drivers; con; con = con->next) {
+ for_each_console(con) {
if ((con->flags & CON_ENABLED) && con->write &&
(cpu_online(smp_processor_id()) ||
(con->flags & CON_ANYTIME)))
@@ -544,7 +550,7 @@ static int have_callable_console(void)
{
struct console *con;
- for (con = console_drivers; con; con = con->next)
+ for_each_console(con)
if (con->flags & CON_ANYTIME)
return 1;
@@ -1082,7 +1088,7 @@ void console_unblank(void)
console_locked = 1;
console_may_schedule = 0;
- for (c = console_drivers; c != NULL; c = c->next)
+ for_each_console(c)
if ((c->flags & CON_ENABLED) && c->unblank)
c->unblank();
release_console_sem();
@@ -1097,7 +1103,7 @@ struct tty_driver *console_device(int *index)
struct tty_driver *driver = NULL;
acquire_console_sem();
- for (c = console_drivers; c != NULL; c = c->next) {
+ for_each_console(c) {
if (!c->device)
continue;
driver = c->device(c, index);
@@ -1135,24 +1141,33 @@ EXPORT_SYMBOL(console_start);
* print any messages that were printed by the kernel before the
* console driver was initialized.
*/
-void register_console(struct console *console)
+void register_console(struct console *newcon)
{
int i;
unsigned long flags;
- struct console *bootconsole = NULL;
+ struct console *bcon = NULL;
+ /*
+ * before we register a new CON_BOOT console, make sure we don't
+ * already have a valid console
+ */
if (console_drivers) {
- if (console->flags & CON_BOOT)
- return;
+ if (newcon->flags & CON_BOOT) {
+ /* find the last or real console */
+ for_each_console(bcon) {
+ if (!(bcon->flags & CON_BOOT))
+ return;
+ }
+ }
if (console_drivers->flags & CON_BOOT)
- bootconsole = console_drivers;
+ bcon = console_drivers;
}
- if (preferred_console < 0 || bootconsole || !console_drivers)
+ if (preferred_console < 0 || bcon || !console_drivers)
preferred_console = selected_console;
- if (console->early_setup)
- console->early_setup();
+ if (newcon->early_setup)
+ newcon->early_setup();
/*
* See if we want to use this console driver. If we
@@ -1160,13 +1175,13 @@ void register_console(struct console *console)
* that registers here.
*/
if (preferred_console < 0) {
- if (console->index < 0)
- console->index = 0;
- if (console->setup == NULL ||
- console->setup(console, NULL) == 0) {
- console->flags |= CON_ENABLED;
- if (console->device) {
- console->flags |= CON_CONSDEV;
+ if (newcon->index < 0)
+ newcon->index = 0;
+ if (newcon->setup == NULL ||
+ newcon->setup(newcon, NULL) == 0) {
+ newcon->flags |= CON_ENABLED;
+ if (newcon->device) {
+ newcon->flags |= CON_CONSDEV;
preferred_console = 0;
}
}
@@ -1178,47 +1193,53 @@ void register_console(struct console *console)
*/
for (i = 0; i < MAX_CMDLINECONSOLES && console_cmdline[i].name[0];
i++) {
- if (strcmp(console_cmdline[i].name, console->name) != 0)
+ if (strcmp(console_cmdline[i].name, newcon->name) != 0)
continue;
- if (console->index >= 0 &&
- console->index != console_cmdline[i].index)
+ if (newcon->index >= 0 &&
+ newcon->index != console_cmdline[i].index)
continue;
- if (console->index < 0)
- console->index = console_cmdline[i].index;
+ if (newcon->index < 0)
+ newcon->index = console_cmdline[i].index;
#ifdef CONFIG_A11Y_BRAILLE_CONSOLE
if (console_cmdline[i].brl_options) {
- console->flags |= CON_BRL;
- braille_register_console(console,
+ newcon->flags |= CON_BRL;
+ braille_register_console(newcon,
console_cmdline[i].index,
console_cmdline[i].options,
console_cmdline[i].brl_options);
return;
}
#endif
- if (console->setup &&
- console->setup(console, console_cmdline[i].options) != 0)
+ if (newcon->setup &&
+ newcon->setup(newcon, console_cmdline[i].options) != 0)
break;
- console->flags |= CON_ENABLED;
- console->index = console_cmdline[i].index;
+ newcon->flags |= CON_ENABLED;
+ newcon->index = console_cmdline[i].index;
if (i == selected_console) {
- console->flags |= CON_CONSDEV;
+ newcon->flags |= CON_CONSDEV;
preferred_console = selected_console;
}
break;
}
- if (!(console->flags & CON_ENABLED))
+ if (!(newcon->flags & CON_ENABLED))
return;
- if (bootconsole && (console->flags & CON_CONSDEV)) {
- 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;
+ if (bcon && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV)) {
+ /* we need to iterate through twice, to make sure we print
+ * everything out, before we unregister the console(s)
+ */
+ printk(KERN_INFO "console handover:");
+ for_each_console(bcon)
+ printk("boot [%s%d] ", bcon->name, bcon->index);
+ printk(" -> real [%s%d]\n", newcon->name, newcon->index);
+ for_each_console(bcon)
+ unregister_console(bcon);
+ newcon->flags &= ~CON_PRINTBUFFER;
} else {
- printk(KERN_INFO "console [%s%d] enabled\n",
- console->name, console->index);
+ printk(KERN_INFO "%sconsole [%s%d] enabled\n",
+ (newcon->flags & CON_BOOT) ? "boot" : "" ,
+ newcon->name, newcon->index);
}
/*
@@ -1226,16 +1247,16 @@ void register_console(struct console *console)
* preferred driver at the head of the list.
*/
acquire_console_sem();
- if ((console->flags & CON_CONSDEV) || console_drivers == NULL) {
- console->next = console_drivers;
- console_drivers = console;
- if (console->next)
- console->next->flags &= ~CON_CONSDEV;
+ if ((newcon->flags & CON_CONSDEV) || console_drivers == NULL) {
+ newcon->next = console_drivers;
+ console_drivers = newcon;
+ if (newcon->next)
+ newcon->next->flags &= ~CON_CONSDEV;
} else {
- console->next = console_drivers->next;
- console_drivers->next = console;
+ newcon->next = console_drivers->next;
+ console_drivers->next = newcon;
}
- if (console->flags & CON_PRINTBUFFER) {
+ if (newcon->flags & CON_PRINTBUFFER) {
/*
* release_console_sem() will print out the buffered messages
* for us.
@@ -1287,11 +1308,13 @@ EXPORT_SYMBOL(unregister_console);
static int __init disable_boot_consoles(void)
{
- if (console_drivers != NULL) {
- if (console_drivers->flags & CON_BOOT) {
+ struct console *con;
+
+ for_each_console(con) {
+ if (con->flags & CON_BOOT) {
printk(KERN_INFO "turn off boot console %s%d\n",
- console_drivers->name, console_drivers->index);
- return unregister_console(console_drivers);
+ con->name, con->index);
+ return unregister_console(con);
}
}
return 0;
On Tue, 30 Jun 2009 22:51:52 -0400
Robin Getz <[email protected]> wrote:
> From: Robin Getz <[email protected]>
>
> Today, register_console() assumes the following usage:
> - The first console to register with a flag set to CON_BOOT is the
> one and only bootconsole.
> - If another register_console() is called with an additional CON_BOOT,
> it is silently rejected.
> - As soon as a console without the CON_BOOT set calls registers
> the bootconsole is automatically unregistered.
> - Once there is a "real" console - register_console() will silently
> reject any consoles with it's CON_BOOT flag set.
hm. I never knew all that. Thanks for taking the time to explain all
this - it helps.
I think it would be useful if we had a description of the new design,
similar to your description of the "Today" behaviour above. Perhaps
as a comment over register_console()?
> In many systems (alpha, blackfin, microblaze, mips, powerpc, sh, & x86), there
> are early_printk implementations, which use the CON_BOOT which come out
> serial ports, vga, usb, & memory buffers. In many embedded systems, it would
> be nice to have two - in case the primary fails, you always have access to a
> backup memory buffer - but this requires at least two CON_BOOT consoles.
>
> This changeset allows multiple boot consoles, and changes the functionality
> to, be mostly the same as the above.
> - Any number CON_BOOT consoles of can be registered
> - A "real" console will unregister all the CON_BOOT consoles
> - Once a "real" console is registered, no more CON_BOOT consoles
> can be added (still silently rejected)
Is the "silent" rejection desirable? Perhaps that's a
programming/configuration error which the developer should be informed
of?
> diff -puN kernel/printk.c~kernel-printkc-handling-more-than-one-con_boot kernel/printk.c
> --- a/kernel/printk.c~kernel-printkc-handling-more-than-one-con_boot
> +++ a/kernel/printk.c
> @@ -37,6 +37,12 @@
> #include <asm/uaccess.h>
>
> /*
> + * for_each_console() allows you to iterate on each console
> + */
> +#define for_each_console(con) \
> + for (con = console_drivers; con != NULL; con = con->next)
hum. Fair enough.
The patch looks good to me.
On Wed 1 Jul 2009 15:44, Andrew Morton pondered:
> On Tue, 30 Jun 2009 22:51:52 -0400
> Robin Getz <[email protected]> wrote:
>
> > From: Robin Getz <[email protected]>
> >
> > Today, register_console() assumes the following usage:
> > - The first console to register with a flag set to CON_BOOT is the
> > one and only bootconsole.
> > - If another register_console() is called with an additional
> > CON_BOOT, it is silently rejected.
> > - As soon as a console without the CON_BOOT set calls registers
> > the bootconsole is automatically unregistered.
> > - Once there is a "real" console - register_console() will silently
> > reject any consoles with it's CON_BOOT flag set.
>
> hm. I never knew all that. Thanks for taking the time to explain all
> this - it helps.
Is it also worthwhile to mention that this function (when used with
early_printk) can get called _very_ early in the boot process, (before
setup_arch() completes)
> I think it would be useful if we had a description of the new design,
> similar to your description of the "Today" behaviour above. Perhaps
> as a comment over register_console()?
Can do - do you want it as v3 or on-top of this one?
> > In many systems (alpha, blackfin, microblaze, mips, powerpc, sh, &
> > x86), there are early_printk implementations, which use the CON_BOOT
> > which come out serial ports, vga, usb, & memory buffers. In many
> > embedded systems, it would be nice to have two - in case the primary
> > fails, you always have access to a backup memory buffer - but this
> > requires at least two CON_BOOT consoles.
> >
> > This changeset allows multiple boot consoles, and changes the
> functionality
> > to, be mostly the same as the above.
> > - Any number CON_BOOT consoles of can be registered
> > - A "real" console will unregister all the CON_BOOT consoles
> > - Once a "real" console is registered, no more CON_BOOT consoles
> > can be added (still silently rejected)
>
> Is the "silent" rejection desirable? Perhaps that's a
> programming/configuration error which the developer should be informed
> of?
That is what it is today. I would actually prefer not silent - as it
would have meant 10 min that I would have saved myself trying to figure out
what was going on... :)
But when I was figuring out how things worked, and had a few too many printks
in register_console - I was getting BUG: "recent printk recursion!" - so I
just left it silent (rather than figuring this out).
>
> > diff -puN
> kernel/printk.c~kernel-printkc-handling-more-than-one-con_boot
> kernel/printk.c
> > --- a/kernel/printk.c~kernel-printkc-handling-more-than-one-con_boot
> > +++ a/kernel/printk.c
> > @@ -37,6 +37,12 @@
> > #include <asm/uaccess.h>
> >
> > /*
> > + * for_each_console() allows you to iterate on each console
> > + */
> > +#define for_each_console(con) \
> > + for (con = console_drivers; con != NULL; con = con->next)
>
> hum. Fair enough.
Was there an issue with this? (Or you just want me to split that up into a
separate patch?)
> The patch looks good to me.
>
On Wed, 1 Jul 2009 16:50:19 -0400
Robin Getz <[email protected]> wrote:
> On Wed 1 Jul 2009 15:44, Andrew Morton pondered:
> > On Tue, 30 Jun 2009 22:51:52 -0400
> > Robin Getz <[email protected]> wrote:
> >
> > > From: Robin Getz <[email protected]>
> > >
> > > Today, register_console() assumes the following usage:
> > > - The first console to register with a flag set to CON_BOOT is the
> > > one and only bootconsole.
> > > - If another register_console() is called with an additional
> > > CON_BOOT, it is silently rejected.
> > > - As soon as a console without the CON_BOOT set calls registers
> > > the bootconsole is automatically unregistered.
> > > - Once there is a "real" console - register_console() will silently
> > > reject any consoles with it's CON_BOOT flag set.
> >
> > hm. I never knew all that. Thanks for taking the time to explain all
> > this - it helps.
>
> Is it also worthwhile to mention that this function (when used with
> early_printk) can get called _very_ early in the boot process, (before
> setup_arch() completes)
OK.
> > I think it would be useful if we had a description of the new design,
> > similar to your description of the "Today" behaviour above. Perhaps
> > as a comment over register_console()?
>
> Can do - do you want it as v3 or on-top of this one?
I'm easy either way. Ingo might have applied to one of his trees, in
which case a replacement patch might cause him problems, dunno.
> > > In many systems (alpha, blackfin, microblaze, mips, powerpc, sh, &
> > > x86), there are early_printk implementations, which use the CON_BOOT
> > > which come out serial ports, vga, usb, & memory buffers. In many
> > > embedded systems, it would be nice to have two - in case the primary
> > > fails, you always have access to a backup memory buffer - but this
> > > requires at least two CON_BOOT consoles.
> > >
> > > This changeset allows multiple boot consoles, and changes the
> > functionality
> > > to, be mostly the same as the above.
> > > - Any number CON_BOOT consoles of can be registered
> > > - A "real" console will unregister all the CON_BOOT consoles
> > > - Once a "real" console is registered, no more CON_BOOT consoles
> > > can be added (still silently rejected)
> >
> > Is the "silent" rejection desirable? Perhaps that's a
> > programming/configuration error which the developer should be informed
> > of?
>
> That is what it is today. I would actually prefer not silent - as it
> would have meant 10 min that I would have saved myself trying to figure out
> what was going on... :)
>
> But when I was figuring out how things worked, and had a few too many printks
> in register_console - I was getting BUG: "recent printk recursion!" - so I
> just left it silent (rather than figuring this out).
OK - I was just wondering. Whatever you think is best..
> >
> > > diff -puN
> > kernel/printk.c~kernel-printkc-handling-more-than-one-con_boot
> > kernel/printk.c
> > > --- a/kernel/printk.c~kernel-printkc-handling-more-than-one-con_boot
> > > +++ a/kernel/printk.c
> > > @@ -37,6 +37,12 @@
> > > #include <asm/uaccess.h>
> > >
> > > /*
> > > + * for_each_console() allows you to iterate on each console
> > > + */
> > > +#define for_each_console(con) \
> > > + for (con = console_drivers; con != NULL; con = con->next)
> >
> > hum. Fair enough.
>
> Was there an issue with this? (Or you just want me to split that up into a
> separate patch?)
Just a general allergy to fancypants macros, especially those which
hide control flow. But this one is straightforward enough.
Formally one should parenthesize `con' here, but if that change is ever
actually necessary then we have other problems, and the macro was
probably a bad idea anyway.
* Andrew Morton <[email protected]> wrote:
> > > I think it would be useful if we had a description of the new
> > > design, similar to your description of the "Today" behaviour
> > > above. Perhaps as a comment over register_console()?
> >
> > Can do - do you want it as v3 or on-top of this one?
>
> I'm easy either way. Ingo might have applied to one of his trees,
> in which case a replacement patch might cause him problems, dunno.
Not yet - i was waiting for feedback from you and Linus. Would be
nice to have a from-scratch v3 that adresses all the review
feedback.
Ingo
On Wed 1 Jul 2009 17:01, Andrew Morton pondered:
> > > > diff -puN
> > > kernel/printk.c~kernel-printkc-handling-more-than-one-con_boot
> > > kernel/printk.c
> > > > ---
> a/kernel/printk.c~kernel-printkc-handling-more-than-one-con_boot
> > > > +++ a/kernel/printk.c
> > > > @@ -37,6 +37,12 @@
> > > > #include <asm/uaccess.h>
> > > >
> > > > /*
> > > > + * for_each_console() allows you to iterate on each console
> > > > + */
> > > > +#define for_each_console(con) \
> > > > + for (con = console_drivers; con != NULL; con = con->next)
> > >
> > > hum. Fair enough.
> >
> > Was there an issue with this? (Or you just want me to split that up
> > into a separate patch?)
>
> Just a general allergy to fancypants macros, especially those which
> hide control flow. But this one is straightforward enough.
>
> Formally one should parenthesize `con' here, but if that change is ever
> actually necessary then we have other problems, and the macro was
> probably a bad idea anyway.
Ingo complained that the first version (without the macro) looked crappy
(my word) because of the 80 col limit...
I borrowed this concept from the other files in ./kernel/ to make things
a little nicer looking.
kernel/kexec.c:613:#define for_each_kimage_entry(image, ptr, entry) \
kernel/kexec.c-614- for (ptr = &image->head; (entry = *ptr) && !(entry & IND_DONE); \
kernel/kexec.c-615- ptr = (entry & IND_INDIRECTION)? \
kernel/sched.c:688:#define for_each_domain(cpu, __sd) \
kernel/sched.c-689- for (__sd = rcu_dereference(cpu_rq(cpu)->sd); __sd; __sd = __sd->parent)
kernel/sched.c:1743:#define for_each_class(class) \
kernel/sched.c-1744- for (class = sched_class_highest; class; class = class->next)
kernel/sched.c:4437:#define for_each_flag_domain(cpu, sd, flag) \
kernel/sched.c-4438- for (sd = lowest_flag_domain(cpu, flag); \
kernel/sched.c-4439- (sd && (sd->flags & flag)); sd = sd->parent)
kernel/sched_cpupri.c:49:#define for_each_cpupri_active(array, idx) \
kernel/sched_cpupri.c-50- for (idx = find_first_bit(array, CPUPRI_NR_PRIORITIES); \
kernel/sched_cpupri.c-51- idx < CPUPRI_NR_PRIORITIES; \
kernel/sched_fair.c:99:#define for_each_sched_entity(se) \
kernel/sched_fair.c-100- for (; se; se = se->parent)
kernel/sched_fair.c:128:#define for_each_leaf_cfs_rq(rq, cfs_rq) \
kernel/sched_fair.c-129- list_for_each_entry_rcu(cfs_rq, &rq->leaf_cfs_rq_list, leaf_cfs_rq_list)
kernel/sched_fair.c:198:#define for_each_sched_entity(se) \
kernel/sched_fair.c-199- for (; se; se = NULL)
kernel/sched_fair.c:225:#define for_each_leaf_cfs_rq(rq, cfs_rq) \
kernel/sched_fair.c-226- for (cfs_rq = &rq->cfs; cfs_rq; cfs_rq = NULL)
kernel/sched_rt.c:157:#define for_each_leaf_rt_rq(rt_rq, rq) \
kernel/sched_rt.c-158- list_for_each_entry_rcu(rt_rq, &rq->leaf_rt_rq_list, leaf_rt_rq_list)
kernel/sched_rt.c:160:#define for_each_sched_rt_entity(rt_se) \
kernel/sched_rt.c-161- for (; rt_se; rt_se = rt_se->parent)
kernel/sched_rt.c:244:#define for_each_leaf_rt_rq(rt_rq, rq) \
kernel/sched_rt.c-245- for (rt_rq = &rq->rt; rt_rq; rt_rq = NULL)
kernel/sched_rt.c:247:#define for_each_sched_rt_entity(rt_se) \
kernel/sched_rt.c-248- for (; rt_se; rt_se = NULL)
I thought it was the "prefered" way to do things.
Guess I was wrong. :)
On Wed 1 Jul 2009 17:01, Andrew Morton pondered:
> > > I think it would be useful if we had a description of the new
> > > design, similar to your description of the "Today" behaviour
> > > above. Perhaps as a comment over register_console()?
> >
> > Can do - do you want it as v3 or on-top of this one?
>
> I'm easy either way. Ingo might have applied to one of his trees, in
> which case a replacement patch might cause him problems, dunno.
Ingo? I'm OK with either way.
> > > > This changeset allows multiple boot consoles, and changes the
> > > > functionality to, be mostly the same as the above.
> > > > - Any number CON_BOOT consoles of can be registered
> > > > - A "real" console will unregister all the CON_BOOT consoles
> > > > - Once a "real" console is registered, no more CON_BOOT consoles
> > > > can be added (still silently rejected)
> > >
> > > Is the "silent" rejection desirable? Perhaps that's a
> > > programming/configuration error which the developer should be
> > > informed of?
> >
> > That is what it is today. I would actually prefer not silent - as it
> > would have meant 10 min that I would have saved myself trying to
> > figure out what was going on... :)
> >
> > But when I was figuring out how things worked, and had a few too many
> > printks in register_console - I was getting BUG: "recent printk
> > recursion!" - so I just left it silent (rather than figuring this out).
>
> OK - I was just wondering. Whatever you think is best..
There is a difference between best and lazy. I picked lazy.
I will see if I can figure it out.
From: Robin Getz <[email protected]>
Today, register_console() assumes the following usage:
- The first console to register with a flag set to CON_BOOT is the
one and only bootconsole.
- If another register_console() is called with an additional CON_BOOT,
it is silently rejected.
- As soon as a console without the CON_BOOT set calls registers
the bootconsole is automatically unregistered.
- Once there is a "real" console - register_console() will silently
reject any consoles with it's CON_BOOT flag set.
In many systems (alpha, blackfin, microblaze, mips, powerpc, sh, & x86), there
are early_printk implementations, which use the CON_BOOT which come out
serial ports, vga, usb, & memory buffers. In many embedded systems, it would
be nice to have two bootconsoles - in case the primary fails, you always have
access to a backup memory buffer - but this requires at least two CON_BOOT
consoles...
This patch enables that functionality.
Signed-off-by : Robin Getz <[email protected]>
---
With the changeset, on boot you get (if you try to re-enable a boot console
after the "real" console has been registered):
root:/> dmesg | grep console
bootconsole [early_shadow0] enabled
bootconsole [early_BFuart0] enabled
Kernel command line: root=/dev/mtdblock0 rw earlyprintk=serial,uart0,57600 console=ttyBF0,57600 nmi_debug=regs
console handover:boot [early_BFuart0] boot [early_shadow0] -> real [ttyBF0]
Too late to register bootconsole early_shadow0
or
root:/> dmesg | grep console
Kernel command line: root=/dev/mtdblock0 rw console=ttyBF0,57600
console [ttyBF0] enabled
printk.c | 146 +++++++++++++++++++++++++++++++++--------------------
1 file changed, 92 insertions(+), 54 deletions(-)
diff --git a/kernel/printk.c b/kernel/printk.c
index b4d97b5..9acc262 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -37,6 +37,12 @@
#include <asm/uaccess.h>
/*
+ * for_each_console() allows you to iterate on each console
+ */
+#define for_each_console(con) \
+ for (con = console_drivers; con != NULL; con = con->next)
+
+/*
* Architectures can override it:
*/
void asmlinkage __attribute__((weak)) early_printk(const char *fmt, ...)
@@ -412,7 +418,7 @@ static void __call_console_drivers(unsigned start, unsigned end)
{
struct console *con;
- for (con = console_drivers; con; con = con->next) {
+ for_each_console(con) {
if ((con->flags & CON_ENABLED) && con->write &&
(cpu_online(smp_processor_id()) ||
(con->flags & CON_ANYTIME)))
@@ -544,7 +550,7 @@ static int have_callable_console(void)
{
struct console *con;
- for (con = console_drivers; con; con = con->next)
+ for_each_console(con)
if (con->flags & CON_ANYTIME)
return 1;
@@ -1082,7 +1088,7 @@ void console_unblank(void)
console_locked = 1;
console_may_schedule = 0;
- for (c = console_drivers; c != NULL; c = c->next)
+ for_each_console(c)
if ((c->flags & CON_ENABLED) && c->unblank)
c->unblank();
release_console_sem();
@@ -1097,7 +1103,7 @@ struct tty_driver *console_device(int *index)
struct tty_driver *driver = NULL;
acquire_console_sem();
- for (c = console_drivers; c != NULL; c = c->next) {
+ for_each_console(c) {
if (!c->device)
continue;
driver = c->device(c, index);
@@ -1134,25 +1140,49 @@ EXPORT_SYMBOL(console_start);
* to register the console printing procedure with printk() and to
* print any messages that were printed by the kernel before the
* console driver was initialized.
+ *
+ * This can happen pretty early during the boot process (because of
+ * early_printk) - sometimes before setup_arch() completes - be careful
+ * of what kernel features are used - they may not be initialised yet.
+ *
+ * There are two types of consoles - bootconsoles (early_printk) and
+ * "real" consoles (everything which is not a bootconsole) which are
+ * handled differently.
+ * - Any number of bootconsoles can be registered at any time.
+ * - As soon as a "real" console is registered, all bootconsoles
+ * will be unregistered automatically.
+ * - Once a "real" console is registered, any attempt to register a
+ * bootconsoles will be rejected
*/
-void register_console(struct console *console)
+void register_console(struct console *newcon)
{
int i;
unsigned long flags;
- struct console *bootconsole = NULL;
+ struct console *bcon = NULL;
- if (console_drivers) {
- if (console->flags & CON_BOOT)
- return;
- if (console_drivers->flags & CON_BOOT)
- bootconsole = console_drivers;
+ /*
+ * before we register a new CON_BOOT console, make sure we don't
+ * already have a valid console
+ */
+ if (console_drivers && newcon->flags & CON_BOOT) {
+ /* find the last or real console */
+ for_each_console(bcon) {
+ if (!(bcon->flags & CON_BOOT)) {
+ printk(KERN_INFO "Too late to register bootconsole %s%d\n",
+ newcon->name, newcon->index);
+ return;
+ }
+ }
}
+
+ if (console_drivers && console_drivers->flags & CON_BOOT)
+ bcon = console_drivers;
- if (preferred_console < 0 || bootconsole || !console_drivers)
+ if (preferred_console < 0 || bcon || !console_drivers)
preferred_console = selected_console;
- if (console->early_setup)
- console->early_setup();
+ if (newcon->early_setup)
+ newcon->early_setup();
/*
* See if we want to use this console driver. If we
@@ -1160,13 +1190,13 @@ void register_console(struct console *console)
* that registers here.
*/
if (preferred_console < 0) {
- if (console->index < 0)
- console->index = 0;
- if (console->setup == NULL ||
- console->setup(console, NULL) == 0) {
- console->flags |= CON_ENABLED;
- if (console->device) {
- console->flags |= CON_CONSDEV;
+ if (newcon->index < 0)
+ newcon->index = 0;
+ if (newcon->setup == NULL ||
+ newcon->setup(newcon, NULL) == 0) {
+ newcon->flags |= CON_ENABLED;
+ if (newcon->device) {
+ newcon->flags |= CON_CONSDEV;
preferred_console = 0;
}
}
@@ -1178,47 +1208,53 @@ void register_console(struct console *console)
*/
for (i = 0; i < MAX_CMDLINECONSOLES && console_cmdline[i].name[0];
i++) {
- if (strcmp(console_cmdline[i].name, console->name) != 0)
+ if (strcmp(console_cmdline[i].name, newcon->name) != 0)
continue;
- if (console->index >= 0 &&
- console->index != console_cmdline[i].index)
+ if (newcon->index >= 0 &&
+ newcon->index != console_cmdline[i].index)
continue;
- if (console->index < 0)
- console->index = console_cmdline[i].index;
+ if (newcon->index < 0)
+ newcon->index = console_cmdline[i].index;
#ifdef CONFIG_A11Y_BRAILLE_CONSOLE
if (console_cmdline[i].brl_options) {
- console->flags |= CON_BRL;
- braille_register_console(console,
+ newcon->flags |= CON_BRL;
+ braille_register_console(newcon,
console_cmdline[i].index,
console_cmdline[i].options,
console_cmdline[i].brl_options);
return;
}
#endif
- if (console->setup &&
- console->setup(console, console_cmdline[i].options) != 0)
+ if (newcon->setup &&
+ newcon->setup(newcon, console_cmdline[i].options) != 0)
break;
- console->flags |= CON_ENABLED;
- console->index = console_cmdline[i].index;
+ newcon->flags |= CON_ENABLED;
+ newcon->index = console_cmdline[i].index;
if (i == selected_console) {
- console->flags |= CON_CONSDEV;
+ newcon->flags |= CON_CONSDEV;
preferred_console = selected_console;
}
break;
}
- if (!(console->flags & CON_ENABLED))
+ if (!(newcon->flags & CON_ENABLED))
return;
- if (bootconsole && (console->flags & CON_CONSDEV)) {
- 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;
+ if (bcon && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV)) {
+ /* we need to iterate through twice, to make sure we print
+ * everything out, before we unregister the console(s)
+ */
+ printk(KERN_INFO "console handover:");
+ for_each_console(bcon)
+ printk("boot [%s%d] ", bcon->name, bcon->index);
+ printk(" -> real [%s%d]\n", newcon->name, newcon->index);
+ for_each_console(bcon)
+ unregister_console(bcon);
+ newcon->flags &= ~CON_PRINTBUFFER;
} else {
- printk(KERN_INFO "console [%s%d] enabled\n",
- console->name, console->index);
+ printk(KERN_INFO "%sconsole [%s%d] enabled\n",
+ (newcon->flags & CON_BOOT) ? "boot" : "" ,
+ newcon->name, newcon->index);
}
/*
@@ -1226,16 +1262,16 @@ void register_console(struct console *console)
* preferred driver at the head of the list.
*/
acquire_console_sem();
- if ((console->flags & CON_CONSDEV) || console_drivers == NULL) {
- console->next = console_drivers;
- console_drivers = console;
- if (console->next)
- console->next->flags &= ~CON_CONSDEV;
+ if ((newcon->flags & CON_CONSDEV) || console_drivers == NULL) {
+ newcon->next = console_drivers;
+ console_drivers = newcon;
+ if (newcon->next)
+ newcon->next->flags &= ~CON_CONSDEV;
} else {
- console->next = console_drivers->next;
- console_drivers->next = console;
+ newcon->next = console_drivers->next;
+ console_drivers->next = newcon;
}
- if (console->flags & CON_PRINTBUFFER) {
+ if (newcon->flags & CON_PRINTBUFFER) {
/*
* release_console_sem() will print out the buffered messages
* for us.
@@ -1287,11 +1323,13 @@ EXPORT_SYMBOL(unregister_console);
static int __init disable_boot_consoles(void)
{
- if (console_drivers != NULL) {
- if (console_drivers->flags & CON_BOOT) {
+ struct console *con;
+
+ for_each_console(con) {
+ if (con->flags & CON_BOOT) {
printk(KERN_INFO "turn off boot console %s%d\n",
- console_drivers->name, console_drivers->index);
- return unregister_console(console_drivers);
+ con->name, con->index);
+ return unregister_console(con);
}
}
return 0;
Commit-ID: 4d09161196c9a836eacea4b36e2f217bc34894cf
Gitweb: http://git.kernel.org/tip/4d09161196c9a836eacea4b36e2f217bc34894cf
Author: Robin Getz <[email protected]>
AuthorDate: Wed, 1 Jul 2009 21:08:37 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 3 Jul 2009 10:10:43 +0200
printk: Enable the use of more than one CON_BOOT (early console)
Today, register_console() assumes the following usage:
- The first console to register with a flag set to CON_BOOT
is the one and only bootconsole.
- If another register_console() is called with an additional
CON_BOOT, it is silently rejected.
- As soon as a console without the CON_BOOT set calls
registers the bootconsole is automatically unregistered.
- Once there is a "real" console - register_console() will
silently reject any consoles with it's CON_BOOT flag set.
In many systems (alpha, blackfin, microblaze, mips, powerpc,
sh, & x86), there are early_printk implementations, which use
the CON_BOOT which come out serial ports, vga, usb, & memory
buffers.
In many embedded systems, it would be nice to have two
bootconsoles - in case the primary fails, you always have
access to a backup memory buffer - but this requires at least
two CON_BOOT consoles...
This patch enables that functionality.
With the change applied, on boot you get (if you try to
re-enable a boot console after the "real" console has been
registered):
root:/> dmesg | grep console
bootconsole [early_shadow0] enabled
bootconsole [early_BFuart0] enabled
Kernel command line: root=/dev/mtdblock0 rw earlyprintk=serial,uart0,57600 console=ttyBF0,57600 nmi_debug=regs
console handover:boot [early_BFuart0] boot [early_shadow0] -> real [ttyBF0]
Too late to register bootconsole early_shadow0
or:
root:/> dmesg | grep console
Kernel command line: root=/dev/mtdblock0 rw console=ttyBF0,57600
console [ttyBF0] enabled
Signed-off-by: Robin Getz <[email protected]>
Cc: "Linus Torvalds" <[email protected]>
Cc: "Andrew Morton" <[email protected]>
Cc: "Mike Frysinger" <[email protected]>
Cc: "Paul Mundt" <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/printk.c | 146 ++++++++++++++++++++++++++++++++++--------------------
1 files changed, 92 insertions(+), 54 deletions(-)
diff --git a/kernel/printk.c b/kernel/printk.c
index b4d97b5..41fe609 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -37,6 +37,12 @@
#include <asm/uaccess.h>
/*
+ * for_each_console() allows you to iterate on each console
+ */
+#define for_each_console(con) \
+ for (con = console_drivers; con != NULL; con = con->next)
+
+/*
* Architectures can override it:
*/
void asmlinkage __attribute__((weak)) early_printk(const char *fmt, ...)
@@ -412,7 +418,7 @@ static void __call_console_drivers(unsigned start, unsigned end)
{
struct console *con;
- for (con = console_drivers; con; con = con->next) {
+ for_each_console(con) {
if ((con->flags & CON_ENABLED) && con->write &&
(cpu_online(smp_processor_id()) ||
(con->flags & CON_ANYTIME)))
@@ -544,7 +550,7 @@ static int have_callable_console(void)
{
struct console *con;
- for (con = console_drivers; con; con = con->next)
+ for_each_console(con)
if (con->flags & CON_ANYTIME)
return 1;
@@ -1082,7 +1088,7 @@ void console_unblank(void)
console_locked = 1;
console_may_schedule = 0;
- for (c = console_drivers; c != NULL; c = c->next)
+ for_each_console(c)
if ((c->flags & CON_ENABLED) && c->unblank)
c->unblank();
release_console_sem();
@@ -1097,7 +1103,7 @@ struct tty_driver *console_device(int *index)
struct tty_driver *driver = NULL;
acquire_console_sem();
- for (c = console_drivers; c != NULL; c = c->next) {
+ for_each_console(c) {
if (!c->device)
continue;
driver = c->device(c, index);
@@ -1134,25 +1140,49 @@ EXPORT_SYMBOL(console_start);
* to register the console printing procedure with printk() and to
* print any messages that were printed by the kernel before the
* console driver was initialized.
+ *
+ * This can happen pretty early during the boot process (because of
+ * early_printk) - sometimes before setup_arch() completes - be careful
+ * of what kernel features are used - they may not be initialised yet.
+ *
+ * There are two types of consoles - bootconsoles (early_printk) and
+ * "real" consoles (everything which is not a bootconsole) which are
+ * handled differently.
+ * - Any number of bootconsoles can be registered at any time.
+ * - As soon as a "real" console is registered, all bootconsoles
+ * will be unregistered automatically.
+ * - Once a "real" console is registered, any attempt to register a
+ * bootconsoles will be rejected
*/
-void register_console(struct console *console)
+void register_console(struct console *newcon)
{
int i;
unsigned long flags;
- struct console *bootconsole = NULL;
+ struct console *bcon = NULL;
- if (console_drivers) {
- if (console->flags & CON_BOOT)
- return;
- if (console_drivers->flags & CON_BOOT)
- bootconsole = console_drivers;
+ /*
+ * before we register a new CON_BOOT console, make sure we don't
+ * already have a valid console
+ */
+ if (console_drivers && newcon->flags & CON_BOOT) {
+ /* find the last or real console */
+ for_each_console(bcon) {
+ if (!(bcon->flags & CON_BOOT)) {
+ printk(KERN_INFO "Too late to register bootconsole %s%d\n",
+ newcon->name, newcon->index);
+ return;
+ }
+ }
}
- if (preferred_console < 0 || bootconsole || !console_drivers)
+ if (console_drivers && console_drivers->flags & CON_BOOT)
+ bcon = console_drivers;
+
+ if (preferred_console < 0 || bcon || !console_drivers)
preferred_console = selected_console;
- if (console->early_setup)
- console->early_setup();
+ if (newcon->early_setup)
+ newcon->early_setup();
/*
* See if we want to use this console driver. If we
@@ -1160,13 +1190,13 @@ void register_console(struct console *console)
* that registers here.
*/
if (preferred_console < 0) {
- if (console->index < 0)
- console->index = 0;
- if (console->setup == NULL ||
- console->setup(console, NULL) == 0) {
- console->flags |= CON_ENABLED;
- if (console->device) {
- console->flags |= CON_CONSDEV;
+ if (newcon->index < 0)
+ newcon->index = 0;
+ if (newcon->setup == NULL ||
+ newcon->setup(newcon, NULL) == 0) {
+ newcon->flags |= CON_ENABLED;
+ if (newcon->device) {
+ newcon->flags |= CON_CONSDEV;
preferred_console = 0;
}
}
@@ -1178,47 +1208,53 @@ void register_console(struct console *console)
*/
for (i = 0; i < MAX_CMDLINECONSOLES && console_cmdline[i].name[0];
i++) {
- if (strcmp(console_cmdline[i].name, console->name) != 0)
+ if (strcmp(console_cmdline[i].name, newcon->name) != 0)
continue;
- if (console->index >= 0 &&
- console->index != console_cmdline[i].index)
+ if (newcon->index >= 0 &&
+ newcon->index != console_cmdline[i].index)
continue;
- if (console->index < 0)
- console->index = console_cmdline[i].index;
+ if (newcon->index < 0)
+ newcon->index = console_cmdline[i].index;
#ifdef CONFIG_A11Y_BRAILLE_CONSOLE
if (console_cmdline[i].brl_options) {
- console->flags |= CON_BRL;
- braille_register_console(console,
+ newcon->flags |= CON_BRL;
+ braille_register_console(newcon,
console_cmdline[i].index,
console_cmdline[i].options,
console_cmdline[i].brl_options);
return;
}
#endif
- if (console->setup &&
- console->setup(console, console_cmdline[i].options) != 0)
+ if (newcon->setup &&
+ newcon->setup(newcon, console_cmdline[i].options) != 0)
break;
- console->flags |= CON_ENABLED;
- console->index = console_cmdline[i].index;
+ newcon->flags |= CON_ENABLED;
+ newcon->index = console_cmdline[i].index;
if (i == selected_console) {
- console->flags |= CON_CONSDEV;
+ newcon->flags |= CON_CONSDEV;
preferred_console = selected_console;
}
break;
}
- if (!(console->flags & CON_ENABLED))
+ if (!(newcon->flags & CON_ENABLED))
return;
- if (bootconsole && (console->flags & CON_CONSDEV)) {
- 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;
+ if (bcon && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV)) {
+ /* we need to iterate through twice, to make sure we print
+ * everything out, before we unregister the console(s)
+ */
+ printk(KERN_INFO "console handover:");
+ for_each_console(bcon)
+ printk("boot [%s%d] ", bcon->name, bcon->index);
+ printk(" -> real [%s%d]\n", newcon->name, newcon->index);
+ for_each_console(bcon)
+ unregister_console(bcon);
+ newcon->flags &= ~CON_PRINTBUFFER;
} else {
- printk(KERN_INFO "console [%s%d] enabled\n",
- console->name, console->index);
+ printk(KERN_INFO "%sconsole [%s%d] enabled\n",
+ (newcon->flags & CON_BOOT) ? "boot" : "" ,
+ newcon->name, newcon->index);
}
/*
@@ -1226,16 +1262,16 @@ void register_console(struct console *console)
* preferred driver at the head of the list.
*/
acquire_console_sem();
- if ((console->flags & CON_CONSDEV) || console_drivers == NULL) {
- console->next = console_drivers;
- console_drivers = console;
- if (console->next)
- console->next->flags &= ~CON_CONSDEV;
+ if ((newcon->flags & CON_CONSDEV) || console_drivers == NULL) {
+ newcon->next = console_drivers;
+ console_drivers = newcon;
+ if (newcon->next)
+ newcon->next->flags &= ~CON_CONSDEV;
} else {
- console->next = console_drivers->next;
- console_drivers->next = console;
+ newcon->next = console_drivers->next;
+ console_drivers->next = newcon;
}
- if (console->flags & CON_PRINTBUFFER) {
+ if (newcon->flags & CON_PRINTBUFFER) {
/*
* release_console_sem() will print out the buffered messages
* for us.
@@ -1287,11 +1323,13 @@ EXPORT_SYMBOL(unregister_console);
static int __init disable_boot_consoles(void)
{
- if (console_drivers != NULL) {
- if (console_drivers->flags & CON_BOOT) {
+ struct console *con;
+
+ for_each_console(con) {
+ if (con->flags & CON_BOOT) {
printk(KERN_INFO "turn off boot console %s%d\n",
- console_drivers->name, console_drivers->index);
- return unregister_console(console_drivers);
+ con->name, con->index);
+ return unregister_console(con);
}
}
return 0;
On Wed 1 Jul 2009 21:08, Robin Getz pondered:
> printk.c | 146 +++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 92 insertions(+), 54 deletions(-)
>
> diff --git a/kernel/printk.c b/kernel/printk.c
> index b4d97b5..9acc262 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
[snip]
> static int __init disable_boot_consoles(void)
> {
> - if (console_drivers != NULL) {
> - if (console_drivers->flags & CON_BOOT) {
> + struct console *con;
> +
> + for_each_console(con) {
> + if (con->flags & CON_BOOT) {
> printk(KERN_INFO "turn off boot console %s%d\n",
> - console_drivers->name, console_drivers->index);
> - return unregister_console(console_drivers);
> + con->name, con->index);
> + return unregister_console(con);
> }
> }
> return 0;
This causes a bug that Sonic found.
The below patch fixes it, and should be applied to tip.
---
From: Sonic Zhang <[email protected]>
Don't return when we find the first bootconsole - it can leave other
bootconsoles still installed, and they can be used and cause problems later
(if they are in the init section, and eventually released), and cause
problems. Make sure we remove all of them.
Signed-off-by: Sonic Zhang <[email protected]>
Signed-off-by: Robin Getz <[email protected]>
---
diff --git a/kernel/printk.c b/kernel/printk.c
index e0daaf5..e10d193 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -1352,7 +1352,7 @@ static int __init disable_boot_consoles(void)
if (con->flags & CON_BOOT) {
printk(KERN_INFO "turn off boot console %s%d\n",
con->name, con->index);
- return unregister_console(con);
+ unregister_console(con);
}
}
return 0;