| Message ID | 20220924000454.3319186-12-john.ogness@linutronix.de (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | preparation for threaded/atomic printing | expand |
On (22/09/24 02:10), John Ogness wrote: [..] > static int console_unregister_locked(struct console *console) > { > - struct console *con; > int res; > > con_printk(KERN_INFO, console, "disabled\n"); > @@ -3274,32 +3287,28 @@ static int console_unregister_locked(struct console *console) > if (res > 0) > return 0; > > - res = -ENODEV; > console_lock(); > - if (console_drivers == console) { > - console_drivers=console->next; > - res = 0; > - } else { > - for_each_console(con) { > - if (con->next == console) { > - con->next = console->next; > - res = 0; > - break; > - } > - } > - } > > - if (res) > - goto out_disable_unlock; > + /* Disable it unconditionally */ > + console->flags &= ~CON_ENABLED; > + > + if (hlist_unhashed(&console->node)) > + goto out_unlock; Shouldn't this set `ret` to -ENODEV before goto? Otherwise it will always return 0 (which is set by _braille_unregister_console()). > + > + hlist_del_init(&console->node); > > /* > + * <HISTORICAL> > * If this isn't the last console and it has CON_CONSDEV set, we > * need to set it on the next preferred console. > + * </HISTORICAL> > + * > + * The above makes no sense as there is no guarantee that the next > + * console has any device attached. Oh well.... > */ It's complicated...
Hi John, * John Ogness <john.ogness@linutronix.de>: > From: Thomas Gleixner <tglx@linutronix.de> > > Replace the open coded single linked list with a hlist so a conversion to > SRCU protected list walks can reuse the existing primitives. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Signed-off-by: John Ogness <john.ogness@linutronix.de> > --- > arch/parisc/kernel/pdc_cons.c | 19 +++---- > fs/proc/consoles.c | 5 +- > include/linux/console.h | 15 ++++-- > kernel/printk/printk.c | 99 +++++++++++++++++++---------------- > 4 files changed, 75 insertions(+), 63 deletions(-) > > diff --git a/arch/parisc/kernel/pdc_cons.c b/arch/parisc/kernel/pdc_cons.c > index 9a0c0932d2f9..3f9abf0263ee 100644 > --- a/arch/parisc/kernel/pdc_cons.c > +++ b/arch/parisc/kernel/pdc_cons.c > @@ -272,15 +267,17 @@ void pdc_console_restart(bool hpmc) > if (pdc_console_initialized) > return; > > - if (!hpmc && console_drivers) > + if (!hpmc && !hlist_empty(&console_list)) > return; > > /* If we've already seen the output, don't bother to print it again */ > - if (console_drivers != NULL) > + if (!hlist_empty(&console_list)) > pdc_cons.flags &= ~CON_PRINTBUFFER; > > - while ((console = console_drivers) != NULL) > - unregister_console(console_drivers); > + while (!hlist_empty(&console_list)) { > + unregister_console(READ_ONCE(hlist_entry(console_list.first, > + struct console, node))); > + } > > /* force registering the pdc console */ > pdc_console_init_force(); Thanks for doing this!! I had to add the hunks below on top of your patch to make it compile and boot sucessfully on parisc. Maybe you could fold those into your patch? Thanks! Helge diff --git a/arch/parisc/kernel/pdc_cons.c b/arch/parisc/kernel/pdc_cons.c index 3f9abf0263ee..f15998aa47a8 100644 --- a/arch/parisc/kernel/pdc_cons.c +++ b/arch/parisc/kernel/pdc_cons.c @@ -262,8 +262,6 @@ void __init pdc_console_init(void) */ void pdc_console_restart(bool hpmc) { - struct console *console; - if (pdc_console_initialized) return; @@ -275,8 +273,8 @@ void pdc_console_restart(bool hpmc) pdc_cons.flags &= ~CON_PRINTBUFFER; while (!hlist_empty(&console_list)) { - unregister_console(READ_ONCE(hlist_entry(console_list.first, - struct console, node))); + unregister_console(hlist_entry(console_list.first, + struct console, node)); } /* force registering the pdc console */
On 9/24/22 02:04, John Ogness wrote: > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -79,20 +79,20 @@ int oops_in_progress; > EXPORT_SYMBOL(oops_in_progress); > > /* > - * console_sem protects the console_drivers list, and also provides > - * serialization for access to the entire console driver system. > + * console_sem protects onsole_list, and also provides serialization for Typo: missing the "c" onsole_list -> console_list Helge
On (22/09/24 19:20), Helge Deller wrote: > diff --git a/arch/parisc/kernel/pdc_cons.c b/arch/parisc/kernel/pdc_cons.c > index 3f9abf0263ee..f15998aa47a8 100644 > --- a/arch/parisc/kernel/pdc_cons.c > +++ b/arch/parisc/kernel/pdc_cons.c > @@ -262,8 +262,6 @@ void __init pdc_console_init(void) > */ > void pdc_console_restart(bool hpmc) > { > - struct console *console; > - > if (pdc_console_initialized) > return; > > @@ -275,8 +273,8 @@ void pdc_console_restart(bool hpmc) > pdc_cons.flags &= ~CON_PRINTBUFFER; > > while (!hlist_empty(&console_list)) { > - unregister_console(READ_ONCE(hlist_entry(console_list.first, > - struct console, node))); > + unregister_console(hlist_entry(console_list.first, > + struct console, node)); In this case we maybe can use cons_first() macro here (perhaps give it a little more clear name (first_console()?) and move to printk.h) and also do READ_ONCE there
On Sat 2022-09-24 02:10:47, John Ogness wrote: > From: Thomas Gleixner <tglx@linutronix.de> > > Replace the open coded single linked list with a hlist so a conversion to > SRCU protected list walks can reuse the existing primitives. > > --- a/arch/parisc/kernel/pdc_cons.c > +++ b/arch/parisc/kernel/pdc_cons.c > @@ -272,15 +267,17 @@ void pdc_console_restart(bool hpmc) > if (pdc_console_initialized) > return; > > - if (!hpmc && console_drivers) > + if (!hpmc && !hlist_empty(&console_list)) > return; > > /* If we've already seen the output, don't bother to print it again */ > - if (console_drivers != NULL) > + if (!hlist_empty(&console_list)) > pdc_cons.flags &= ~CON_PRINTBUFFER; > > - while ((console = console_drivers) != NULL) > - unregister_console(console_drivers); > + while (!hlist_empty(&console_list)) { > + unregister_console(READ_ONCE(hlist_entry(console_list.first, > + struct console, node))); The READ_ONCE() is in a wrong place. This is why it did not compile. It should be: unregister_console(hlist_entry(READ_ONCE(console_list.first), struct console, node)); I know that it is all hope for good. But there is also a race between the hlist_empty() and hlist_entry(). We might make it sligtly more safe by using hlist_entry_safe() struct console *con; while (con = hlist_entry_safe(READ_ONCE(console_list.first), struct console, node)) { unregister_console(con); } or while (tmp = READ_ONCE(console_list.first) { unregister_console(hlist_entry_safe(tmp, struct console, node)); } > + } > > /* force registering the pdc console */ > pdc_console_init_force(); > diff --git a/fs/proc/consoles.c b/fs/proc/consoles.c > index 6775056eecd5..70994d1e52f6 100644 > --- a/fs/proc/consoles.c > +++ b/fs/proc/consoles.c > @@ -74,8 +74,11 @@ static void *c_start(struct seq_file *m, loff_t *pos) > static void *c_next(struct seq_file *m, void *v, loff_t *pos) > { > struct console *con = v; > + > ++*pos; > - return con->next; > + hlist_for_each_entry_continue(con, node) > + break; Nit: It looks weird and hacky. It does not look like a common patter. I see that another code reads the next entry instead. I would rather do: return hlist_entry_safe(con->node.next, struct *console, node); and we should later make it rcu safe, something like: return hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(con, struct *console, node)); But I do not have strong opinion. > + return con; > } > > static void c_stop(struct seq_file *m, void *v) > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -2979,7 +2984,15 @@ void console_flush_on_panic(enum con_flush_mode mode) > u64 seq; > > seq = prb_first_valid_seq(prb); > - for_each_console(c) > + /* > + * This cannot use for_each_console() because it's not established > + * that the current context has console locked and neither there is > + * a guarantee that there is no concurrency in that case. > + * > + * Open code it for documentation purposes and pretend that > + * it works. > + */ > + hlist_for_each_entry(c, &console_list, node) > c->seq = seq; It is not a big deal. But I would use the _safe() variant to make it slightly more robust. > } > console_unlock(); > @@ -3211,21 +3227,17 @@ void register_console(struct console *newcon) > } > > /* > - * Put this console in the list - keep the > - * preferred driver at the head of the list. > + * Put this console in the list and keep the referred driver at the > + * head of the list. > */ > console_lock(); > - if ((newcon->flags & CON_CONSDEV) || console_drivers == NULL) { > - newcon->next = console_drivers; > - console_drivers = newcon; > - if (newcon->next) > - newcon->next->flags &= ~CON_CONSDEV; > - /* Ensure this flag is always set for the head of the list */ > - newcon->flags |= CON_CONSDEV; > - } else { > - newcon->next = console_drivers->next; > - console_drivers->next = newcon; > - } > + if (newcon->flags & CON_CONSDEV || hlist_empty(&console_list)) > + hlist_add_head(&newcon->node, &console_list); > + else > + hlist_add_behind(&newcon->node, console_list.first); > + > + /* Ensure this flag is always set for the head of the list */ > + cons_first()->flags |= CON_CONSDEV; The patch removed: if (newcon->next) newcon->next->flags &= ~CON_CONSDEV; As a result, all consoles will have CON_CONSDEV flag set. We need to remove it in the 2nd console when exists. See below for more details. > newcon->dropped = 0; > if (newcon->flags & CON_PRINTBUFFER) { > @@ -3263,7 +3277,6 @@ EXPORT_SYMBOL(register_console); > > static int console_unregister_locked(struct console *console) > { > - struct console *con; > int res; > > con_printk(KERN_INFO, console, "disabled\n"); > @@ -3274,32 +3287,28 @@ static int console_unregister_locked(struct console *console) > if (res > 0) > return 0; > > - res = -ENODEV; > console_lock(); > - if (console_drivers == console) { > - console_drivers=console->next; > - res = 0; > - } else { > - for_each_console(con) { > - if (con->next == console) { > - con->next = console->next; > - res = 0; > - break; > - } > - } > - } > > - if (res) > - goto out_disable_unlock; > + /* Disable it unconditionally */ > + console->flags &= ~CON_ENABLED; > + > + if (hlist_unhashed(&console->node)) > + goto out_unlock; We should return -ENODEV here. I think that Sergey found this as well. > + hlist_del_init(&console->node); > > /* > + * <HISTORICAL> > * If this isn't the last console and it has CON_CONSDEV set, we > * need to set it on the next preferred console. > + * </HISTORICAL> > + * > + * The above makes no sense as there is no guarantee that the next > + * console has any device attached. Oh well.... This is a sad story. CON_CONSDEV used to be an implementation detail. It was used to associate the preferred console (last on the command line) with /dev/console. It was achieved by putting it at the beginning of the list. All consoled had tty binding at that time. The problem started when the flags became readable by user space via /proc/consoles. There is even a tool (showconsole) that is reading it. As a result people wanted to show correct value. The problem is that con->device never exist during boot. The consoles are registered before the tty subsystem is initialized. I have a patch that sets the flag correctly in console_device() that is called from tty_lookup_driver(). But it is part of a bigger clean up patchset that is sitting in my drawer :-/ On the other hand, the current code kind of works. Most console drivers have the tty binding. I can't recall what is the exception. Maybe boot consoles? > */ > - if (console_drivers != NULL && console->flags & CON_CONSDEV) > - console_drivers->flags |= CON_CONSDEV; > + if (!hlist_empty(&console_list) && console->flags & CON_CONSDEV) > + cons_first()->flags |= CON_CONSDEV; > > > - console->flags &= ~CON_ENABLED; > console_unlock(); > console_sysfs_notify(); Best Regards, Petr
On 9/30/22 16:20, Petr Mladek wrote: > On Sat 2022-09-24 02:10:47, John Ogness wrote: >> From: Thomas Gleixner <tglx@linutronix.de> >> >> Replace the open coded single linked list with a hlist so a conversion to >> SRCU protected list walks can reuse the existing primitives. >> >> --- a/arch/parisc/kernel/pdc_cons.c >> +++ b/arch/parisc/kernel/pdc_cons.c >> @@ -272,15 +267,17 @@ void pdc_console_restart(bool hpmc) >> if (pdc_console_initialized) >> return; >> >> - if (!hpmc && console_drivers) >> + if (!hpmc && !hlist_empty(&console_list)) >> return; >> >> /* If we've already seen the output, don't bother to print it again */ >> - if (console_drivers != NULL) >> + if (!hlist_empty(&console_list)) >> pdc_cons.flags &= ~CON_PRINTBUFFER; >> >> - while ((console = console_drivers) != NULL) >> - unregister_console(console_drivers); >> + while (!hlist_empty(&console_list)) { >> + unregister_console(READ_ONCE(hlist_entry(console_list.first, >> + struct console, node))); > > The READ_ONCE() is in a wrong place. This is why it did not compile. > It should be: > > unregister_console(hlist_entry(READ_ONCE(console_list.first), > struct console, > node)); > > I know that it is all hope for good. But there is also a race between > the hlist_empty() and hlist_entry(). I wonder if pdc_console() is still needed as it is today. When this was written, early_console and such didn't worked for parisc as it should. That's proably why we have this register/unregister in here. Would it make sense, and would we gain something for this printk-series, if I'd try to convert pdc_console to a standard earlycon or earlyprintk device? Helge
On 2022-09-30, Helge Deller <deller@gmx.de> wrote: >> I know that it is all hope for good. But there is also a race between >> the hlist_empty() and hlist_entry(). > > I wonder if pdc_console() is still needed as it is today. When this > was written, early_console and such didn't worked for parisc as it > should. That's proably why we have this register/unregister in here. > > Would it make sense, and would we gain something for this > printk-series, if I'd try to convert pdc_console to a standard > earlycon or earlyprintk device? Having an earlycon or earlyprintk device will not really help you here since those drivers will have already unregistered. However, once we get the new atomic/kthread interface available, it certainly would be useful to implement the pdc_console as an atomic console. John Ogness
* John Ogness <john.ogness@linutronix.de>: > On 2022-09-30, Helge Deller <deller@gmx.de> wrote: > >> I know that it is all hope for good. But there is also a race between > >> the hlist_empty() and hlist_entry(). > > > > I wonder if pdc_console() is still needed as it is today. When this > > was written, early_console and such didn't worked for parisc as it > > should. That's proably why we have this register/unregister in here. > > > > Would it make sense, and would we gain something for this > > printk-series, if I'd try to convert pdc_console to a standard > > earlycon or earlyprintk device? > > Having an earlycon or earlyprintk device will not really help you here > since those drivers will have already unregistered. > > However, once we get the new atomic/kthread interface available, it > certainly would be useful to implement the pdc_console as an atomic > console. My idea was to drop most of the pdc console, so that patch #8 and parts of patch #11 of the printk patch series could be dropped and you won't need to take care of those parts when introducing the printk threaded/atomic printing changes. See patch below. Basically it drops all of the offending code. I haven't yet checked it into my parisc for-next tree to not break something. Helge From 5b697874e10729136ce7dd7b362b276f35fae56d Mon Sep 17 00:00:00 2001 From: Helge Deller <deller@gmx.de> Date: Sat, 1 Oct 2022 00:32:07 +0200 Subject: [PATCH] parisc: Drop PDC console and convert it to an early console Rewrite the PDC console to become an early console, which can be used for kgdb as well. Signed-off-by: Helge Deller <deller@gmx.de> diff --git a/arch/parisc/include/asm/pdc.h b/arch/parisc/include/asm/pdc.h index b643092d4b98..fcbcf9a96c11 100644 --- a/arch/parisc/include/asm/pdc.h +++ b/arch/parisc/include/asm/pdc.h @@ -19,9 +19,6 @@ extern unsigned long parisc_pat_pdc_cap; /* PDC capabilities (PAT) */ #define PDC_TYPE_SYSTEM_MAP 1 /* 32-bit, but supports PDC_SYSTEM_MAP */ #define PDC_TYPE_SNAKE 2 /* Doesn't support SYSTEM_MAP */ -void pdc_console_init(void); /* in pdc_console.c */ -void pdc_console_restart(void); - void setup_pdc(void); /* in inventory.c */ /* wrapper-functions from pdc.c */ diff --git a/arch/parisc/kernel/pdc_cons.c b/arch/parisc/kernel/pdc_cons.c index 2661cdd256ae..45a4d2994857 100644 --- a/arch/parisc/kernel/pdc_cons.c +++ b/arch/parisc/kernel/pdc_cons.c @@ -1,46 +1,24 @@ // SPDX-License-Identifier: GPL-2.0-or-later /* - * PDC Console support - ie use firmware to dump text via boot console + * PDC early console support - ie use firmware to dump text via boot console * - * Copyright (C) 1999-2003 Matthew Wilcox <willy at parisc-linux.org> - * Copyright (C) 2000 Martin K Petersen <mkp at mkp.net> - * Copyright (C) 2000 John Marvin <jsm at parisc-linux.org> - * Copyright (C) 2000-2003 Paul Bame <bame at parisc-linux.org> - * Copyright (C) 2000 Philipp Rumpf <prumpf with tux.org> - * Copyright (C) 2000 Michael Ang <mang with subcarrier.org> - * Copyright (C) 2000 Grant Grundler <grundler with parisc-linux.org> - * Copyright (C) 2001-2002 Ryan Bradetich <rbrad at parisc-linux.org> - * Copyright (C) 2001 Helge Deller <deller at parisc-linux.org> - * Copyright (C) 2001 Thomas Bogendoerfer <tsbogend at parisc-linux.org> - * Copyright (C) 2002 Randolph Chung <tausq with parisc-linux.org> - * Copyright (C) 2010 Guy Martin <gmsoft at tuxicoman.be> + * Copyright (C) 2001-2022 Helge Deller <deller@gmx.de> */ /* - * The PDC console is a simple console, which can be used for debugging - * boot related problems on HP PA-RISC machines. It is also useful when no - * other console works. - * * This code uses the ROM (=PDC) based functions to read and write characters * from and to PDC's boot path. */ -/* Define EARLY_BOOTUP_DEBUG to debug kernel related boot problems. - * On production kernels EARLY_BOOTUP_DEBUG should be undefined. */ -#define EARLY_BOOTUP_DEBUG - - #include <linux/kernel.h> #include <linux/console.h> -#include <linux/string.h> #include <linux/init.h> -#include <linux/major.h> -#include <linux/tty.h> +#include <linux/serial_core.h> +#include <linux/kgdb.h> #include <asm/page.h> /* for PAGE0 */ #include <asm/pdc.h> /* for iodc_call() proto and friends */ static DEFINE_SPINLOCK(pdc_console_lock); -static struct console pdc_cons; static void pdc_console_write(struct console *co, const char *s, unsigned count) { @@ -54,210 +32,47 @@ static void pdc_console_write(struct console *co, const char *s, unsigned count) spin_unlock_irqrestore(&pdc_console_lock, flags); } -int pdc_console_poll_key(struct console *co) +static int kgdb_pdc_read_char(void) { - int c; unsigned long flags; + int c; spin_lock_irqsave(&pdc_console_lock, flags); c = pdc_iodc_getc(); spin_unlock_irqrestore(&pdc_console_lock, flags); - return c; -} - -static int pdc_console_setup(struct console *co, char *options) -{ - return 0; -} - -#if defined(CONFIG_PDC_CONSOLE) -#include <linux/vt_kern.h> -#include <linux/tty_flip.h> - -#define PDC_CONS_POLL_DELAY (30 * HZ / 1000) - -static void pdc_console_poll(struct timer_list *unused); -static DEFINE_TIMER(pdc_console_timer, pdc_console_poll); -static struct tty_port tty_port; - -static int pdc_console_tty_open(struct tty_struct *tty, struct file *filp) -{ - tty_port_tty_set(&tty_port, tty); - mod_timer(&pdc_console_timer, jiffies + PDC_CONS_POLL_DELAY); - - return 0; -} - -static void pdc_console_tty_close(struct tty_struct *tty, struct file *filp) -{ - if (tty->count == 1) { - del_timer_sync(&pdc_console_timer); - tty_port_tty_set(&tty_port, NULL); - } + return (c <= 0) ? NO_POLL_CHAR : c; } -static int pdc_console_tty_write(struct tty_struct *tty, const unsigned char *buf, int count) +static void kgdb_pdc_write_char(u8 chr) { - pdc_console_write(NULL, buf, count); - return count; + if (PAGE0->mem_cons.cl_class != CL_DUPLEX) + pdc_console_write(NULL, &chr, 1); } -static unsigned int pdc_console_tty_write_room(struct tty_struct *tty) -{ - return 32768; /* no limit, no buffer used */ -} - -static const struct tty_operations pdc_console_tty_ops = { - .open = pdc_console_tty_open, - .close = pdc_console_tty_close, - .write = pdc_console_tty_write, - .write_room = pdc_console_tty_write_room, +static struct kgdb_io kgdb_pdc_io_ops = { + .name = "kgdb_pdc", + .read_char = kgdb_pdc_read_char, + .write_char = kgdb_pdc_write_char, }; -static void pdc_console_poll(struct timer_list *unused) -{ - int data, count = 0; - - while (1) { - data = pdc_console_poll_key(NULL); - if (data == -1) - break; - tty_insert_flip_char(&tty_port, data & 0xFF, TTY_NORMAL); - count ++; - } - - if (count) - tty_flip_buffer_push(&tty_port); - - if (pdc_cons.flags & CON_ENABLED) - mod_timer(&pdc_console_timer, jiffies + PDC_CONS_POLL_DELAY); -} - -static struct tty_driver *pdc_console_tty_driver; - -static int __init pdc_console_tty_driver_init(void) -{ - struct tty_driver *driver; - int err; - - /* Check if the console driver is still registered. - * It is unregistered if the pdc console was not selected as the - * primary console. */ - - struct console *tmp; - - console_lock(); - for_each_console(tmp) - if (tmp == &pdc_cons) - break; - console_unlock(); - - if (!tmp) { - printk(KERN_INFO "PDC console driver not registered anymore, not creating %s\n", pdc_cons.name); - return -ENODEV; - } - - printk(KERN_INFO "The PDC console driver is still registered, removing CON_BOOT flag\n"); - pdc_cons.flags &= ~CON_BOOT; - - driver = tty_alloc_driver(1, TTY_DRIVER_REAL_RAW | - TTY_DRIVER_RESET_TERMIOS); - if (IS_ERR(driver)) - return PTR_ERR(driver); - - tty_port_init(&tty_port); - - driver->driver_name = "pdc_cons"; - driver->name = "ttyB"; - driver->major = MUX_MAJOR; - driver->minor_start = 0; - driver->type = TTY_DRIVER_TYPE_SYSTEM; - driver->init_termios = tty_std_termios; - tty_set_operations(driver, &pdc_console_tty_ops); - tty_port_link_device(&tty_port, driver, 0); - - err = tty_register_driver(driver); - if (err) { - printk(KERN_ERR "Unable to register the PDC console TTY driver\n"); - tty_port_destroy(&tty_port); - tty_driver_kref_put(driver); - return err; - } - - pdc_console_tty_driver = driver; - - return 0; -} -device_initcall(pdc_console_tty_driver_init); - -static struct tty_driver * pdc_console_device (struct console *c, int *index) +static int __init pdc_earlycon_setup(struct earlycon_device *device, + const char *opt) { - *index = c->index; - return pdc_console_tty_driver; -} -#else -#define pdc_console_device NULL -#endif - -static struct console pdc_cons = { - .name = "ttyB", - .write = pdc_console_write, - .device = pdc_console_device, - .setup = pdc_console_setup, - .flags = CON_BOOT | CON_PRINTBUFFER, - .index = -1, -}; + struct console *earlycon_console; -static int pdc_console_initialized; - -static void pdc_console_init_force(void) -{ - if (pdc_console_initialized) - return; - ++pdc_console_initialized; - /* If the console is duplex then copy the COUT parameters to CIN. */ if (PAGE0->mem_cons.cl_class == CL_DUPLEX) memcpy(&PAGE0->mem_kbd, &PAGE0->mem_cons, sizeof(PAGE0->mem_cons)); - /* register the pdc console */ - register_console(&pdc_cons); -} - -void __init pdc_console_init(void) -{ -#if defined(EARLY_BOOTUP_DEBUG) || defined(CONFIG_PDC_CONSOLE) - pdc_console_init_force(); -#endif -#ifdef EARLY_BOOTUP_DEBUG - printk(KERN_INFO "Initialized PDC Console for debugging.\n"); -#endif -} - - -/* - * Used for emergencies. Currently only used if an HPMC occurs. If an - * HPMC occurs, it is possible that the current console may not be - * properly initialised after the PDC IO reset. This routine unregisters - * all of the current consoles, reinitializes the pdc console and - * registers it. - */ - -void pdc_console_restart(void) -{ - struct console *console; + earlycon_console = device->con; + earlycon_console->write = pdc_console_write; + device->port.iotype = UPIO_MEM32BE; - if (pdc_console_initialized) - return; + if (IS_ENABLED(CONFIG_KGDB)) + kgdb_register_io_module(&kgdb_pdc_io_ops); - /* If we've already seen the output, don't bother to print it again */ - if (console_drivers != NULL) - pdc_cons.flags &= ~CON_PRINTBUFFER; - - while ((console = console_drivers) != NULL) - unregister_console(console_drivers); - - /* force registering the pdc console */ - pdc_console_init_force(); + return 0; } + +EARLYCON_DECLARE(pdc, pdc_earlycon_setup); diff --git a/arch/parisc/kernel/setup.c b/arch/parisc/kernel/setup.c index f005ddedb50e..375f38d6e1a4 100644 --- a/arch/parisc/kernel/setup.c +++ b/arch/parisc/kernel/setup.c @@ -70,6 +70,10 @@ void __init setup_cmdline(char **cmdline_p) strlcat(p, "tty0", COMMAND_LINE_SIZE); } + /* default to use early console */ + if (!strstr(p, "earlycon")) + strlcat(p, " earlycon=pdc", COMMAND_LINE_SIZE); + #ifdef CONFIG_BLK_DEV_INITRD if (boot_args[2] != 0) /* did palo pass us a ramdisk? */ { @@ -139,8 +143,6 @@ void __init setup_arch(char **cmdline_p) if (__pa((unsigned long) &_end) >= KERNEL_INITIAL_SIZE) panic("KERNEL_INITIAL_ORDER too small!"); - pdc_console_init(); - #ifdef CONFIG_64BIT if(parisc_narrow_firmware) { printk(KERN_INFO "Kernel is using PDC in 32-bit mode.\n"); diff --git a/arch/parisc/kernel/traps.c b/arch/parisc/kernel/traps.c index b78f1b9d45c1..f9696fbf646c 100644 --- a/arch/parisc/kernel/traps.c +++ b/arch/parisc/kernel/traps.c @@ -239,13 +239,6 @@ void die_if_kernel(char *str, struct pt_regs *regs, long err) /* unlock the pdc lock if necessary */ pdc_emergency_unlock(); - /* maybe the kernel hasn't booted very far yet and hasn't been able - * to initialize the serial or STI console. In that case we should - * re-enable the pdc console, so that the user will be able to - * identify the problem. */ - if (!console_drivers) - pdc_console_restart(); - if (err) printk(KERN_CRIT "%s (pid %d): %s (code %ld)\n", current->comm, task_pid_nr(current), str, err); @@ -429,10 +422,6 @@ void parisc_terminate(char *msg, struct pt_regs *regs, int code, unsigned long o /* unlock the pdc lock if necessary */ pdc_emergency_unlock(); - /* restart pdc console if necessary */ - if (!console_drivers) - pdc_console_restart(); - /* Not all paths will gutter the processor... */ switch(code){ @@ -482,9 +471,7 @@ void notrace handle_interruption(int code, struct pt_regs *regs) unsigned long fault_space = 0; int si_code; - if (code == 1) - pdc_console_restart(); /* switch back to pdc if HPMC */ - else if (!irqs_disabled_flags(regs->gr[0])) + if (!irqs_disabled_flags(regs->gr[0])) local_irq_enable(); /* Security check: diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig index 877173907c53..898728ab2c18 100644 --- a/drivers/tty/serial/Kconfig +++ b/drivers/tty/serial/Kconfig @@ -602,21 +602,6 @@ config SERIAL_MUX_CONSOLE select SERIAL_CORE_CONSOLE default y -config PDC_CONSOLE - bool "PDC software console support" - depends on PARISC && !SERIAL_MUX && VT - help - Saying Y here will enable the software based PDC console to be - used as the system console. This is useful for machines in - which the hardware based console has not been written yet. The - following steps must be completed to use the PDC console: - - 1. create the device entry (mknod /dev/ttyB0 c 11 0) - 2. Edit the /etc/inittab to start a getty listening on /dev/ttyB0 - 3. Add device ttyB0 to /etc/securetty (if you want to log on as - root on this console.) - 4. Change the kernel command console parameter to: console=ttyB0 - config SERIAL_SUNSAB tristate "Sun Siemens SAB82532 serial support" depends on SPARC && PCI diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb index 05dae05b6cc9..3b9a44008433 100644 --- a/lib/Kconfig.kgdb +++ b/lib/Kconfig.kgdb @@ -121,7 +121,7 @@ config KDB_DEFAULT_ENABLE config KDB_KEYBOARD bool "KGDB_KDB: keyboard as input device" - depends on VT && KGDB_KDB + depends on VT && KGDB_KDB && !PARISC default n help KDB can use a PS/2 type keyboard for an input device
diff --git a/arch/parisc/kernel/pdc_cons.c b/arch/parisc/kernel/pdc_cons.c index 9a0c0932d2f9..3f9abf0263ee 100644 --- a/arch/parisc/kernel/pdc_cons.c +++ b/arch/parisc/kernel/pdc_cons.c @@ -147,13 +147,8 @@ static int __init pdc_console_tty_driver_init(void) struct console *tmp; - console_lock(); - for_each_console(tmp) - if (tmp == &pdc_cons) - break; - console_unlock(); - - if (!tmp) { + /* Pretend that this works as much as it pretended to work historically */ + if (hlist_unhashed_lockless(&pdc_cons.node)) { printk(KERN_INFO "PDC console driver not registered anymore, not creating %s\n", pdc_cons.name); return -ENODEV; } @@ -272,15 +267,17 @@ void pdc_console_restart(bool hpmc) if (pdc_console_initialized) return; - if (!hpmc && console_drivers) + if (!hpmc && !hlist_empty(&console_list)) return; /* If we've already seen the output, don't bother to print it again */ - if (console_drivers != NULL) + if (!hlist_empty(&console_list)) pdc_cons.flags &= ~CON_PRINTBUFFER; - while ((console = console_drivers) != NULL) - unregister_console(console_drivers); + while (!hlist_empty(&console_list)) { + unregister_console(READ_ONCE(hlist_entry(console_list.first, + struct console, node))); + } /* force registering the pdc console */ pdc_console_init_force(); diff --git a/fs/proc/consoles.c b/fs/proc/consoles.c index 6775056eecd5..70994d1e52f6 100644 --- a/fs/proc/consoles.c +++ b/fs/proc/consoles.c @@ -74,8 +74,11 @@ static void *c_start(struct seq_file *m, loff_t *pos) static void *c_next(struct seq_file *m, void *v, loff_t *pos) { struct console *con = v; + ++*pos; - return con->next; + hlist_for_each_entry_continue(con, node) + break; + return con; } static void c_stop(struct seq_file *m, void *v) diff --git a/include/linux/console.h b/include/linux/console.h index 86a6125512b9..1e3d0a50cef1 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -15,6 +15,7 @@ #define _LINUX_CONSOLE_H_ 1 #include <linux/atomic.h> +#include <linux/list.h> #include <linux/types.h> struct vc_data; @@ -154,15 +155,19 @@ struct console { u64 seq; unsigned long dropped; void *data; - struct console *next; + struct hlist_node node; }; #ifdef CONFIG_LOCKDEP +extern void lockdep_assert_console_lock_held(void); extern void lockdep_assert_console_list_lock_held(void); #else +static inline void lockdep_assert_console_lock_held(void) { } static inline void lockdep_assert_console_list_lock_held(void) { } #endif +extern struct hlist_head console_list; + extern void console_list_lock(void) __acquires(console_mutex); extern void console_list_unlock(void) __releases(console_mutex); @@ -175,7 +180,7 @@ extern void console_list_unlock(void) __releases(console_mutex); */ #define for_each_registered_console(con) \ lockdep_assert_console_list_lock_held(); \ - for (con = console_drivers; con != NULL; con = con->next) + hlist_for_each_entry(con, &console_list, node) /** * for_each_console() - Iterator over registered consoles @@ -185,7 +190,8 @@ extern void console_list_unlock(void) __releases(console_mutex); * list is immutable. */ #define for_each_console(con) \ - for (con = console_drivers; con != NULL; con = con->next) + lockdep_assert_console_lock_held(); \ + hlist_for_each_entry(con, &console_list, node) /** * for_each_console_kgdb() - Iterator over registered consoles for KGDB @@ -195,7 +201,7 @@ extern void console_list_unlock(void) __releases(console_mutex); * Don't use outside of the KGDB fairy tale land! */ #define for_each_console_kgdb(con) \ - for (con = console_drivers; con != NULL; con = con->next) + hlist_for_each_entry(con, &console_list, node) extern int console_set_on_cmdline; extern struct console *early_console; @@ -208,7 +214,6 @@ enum con_flush_mode { extern int add_preferred_console(char *name, int idx, char *options); extern void register_console(struct console *); extern int unregister_console(struct console *); -extern struct console *console_drivers; extern void console_lock(void); extern int console_trylock(void); extern void console_unlock(void); diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 80a728ef9d96..f1d31dcbd6ba 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -79,20 +79,20 @@ int oops_in_progress; EXPORT_SYMBOL(oops_in_progress); /* - * console_sem protects the console_drivers list, and also provides - * serialization for access to the entire console driver system. + * console_sem protects onsole_list, and also provides serialization for + * access to the entire console driver system. * * console_mutex serializes register/unregister. * * console_sem must be taken inside a console_mutex locked section - * for any list manipulation in order to keep the console BKL + * for any console_list manipulation in order to keep the console BKL * machinery happy. This requirement also applies to manipulation * of console->flags. */ static DEFINE_MUTEX(console_mutex); static DEFINE_SEMAPHORE(console_sem); -struct console *console_drivers; -EXPORT_SYMBOL_GPL(console_drivers); +HLIST_HEAD(console_list); +EXPORT_SYMBOL_GPL(console_list); /* * System may need to suppress printk message under certain @@ -111,6 +111,11 @@ static struct lockdep_map console_lock_dep_map = { .name = "console_lock" }; +void lockdep_assert_console_lock_held(void) +{ + lockdep_assert(lock_is_held(&console_lock_dep_map)); +} + void lockdep_assert_console_list_lock_held(void) { lockdep_assert_held(&console_mutex); @@ -2591,7 +2596,7 @@ static int console_cpu_notify(unsigned int cpu) * console_lock - lock the console system for exclusive use. * * Acquires a lock which guarantees that the caller has - * exclusive access to the console system and the console_drivers list. + * exclusive access to the console system and console_list. * * Can sleep, returns nothing. */ @@ -2611,7 +2616,7 @@ EXPORT_SYMBOL(console_lock); * console_trylock - try to lock the console system for exclusive use. * * Try to acquire a lock which guarantees that the caller has exclusive - * access to the console system and the console_drivers list. + * access to the console system and console_list. * * returns 1 on success, and 0 on failure to acquire the lock. */ @@ -2979,7 +2984,15 @@ void console_flush_on_panic(enum con_flush_mode mode) u64 seq; seq = prb_first_valid_seq(prb); - for_each_console(c) + /* + * This cannot use for_each_console() because it's not established + * that the current context has console locked and neither there is + * a guarantee that there is no concurrency in that case. + * + * Open code it for documentation purposes and pretend that + * it works. + */ + hlist_for_each_entry(c, &console_list, node) c->seq = seq; } console_unlock(); @@ -3120,6 +3133,9 @@ static void try_enable_default_console(struct console *newcon) (con->flags & CON_BOOT) ? "boot" : "", \ con->name, con->index, ##__VA_ARGS__) +#define cons_first() \ + hlist_entry(console_list.first, struct console, node) + static int console_unregister_locked(struct console *console); /* @@ -3182,8 +3198,8 @@ void register_console(struct console *newcon) * flag set and will be first in the list. */ if (preferred_console < 0) { - if (!console_drivers || !console_drivers->device || - console_drivers->flags & CON_BOOT) { + if (hlist_empty(&console_list) || !cons_first()->device || + cons_first()->flags & CON_BOOT) { try_enable_default_console(newcon); } } @@ -3211,21 +3227,17 @@ void register_console(struct console *newcon) } /* - * Put this console in the list - keep the - * preferred driver at the head of the list. + * Put this console in the list and keep the referred driver at the + * head of the list. */ console_lock(); - if ((newcon->flags & CON_CONSDEV) || console_drivers == NULL) { - newcon->next = console_drivers; - console_drivers = newcon; - if (newcon->next) - newcon->next->flags &= ~CON_CONSDEV; - /* Ensure this flag is always set for the head of the list */ - newcon->flags |= CON_CONSDEV; - } else { - newcon->next = console_drivers->next; - console_drivers->next = newcon; - } + if (newcon->flags & CON_CONSDEV || hlist_empty(&console_list)) + hlist_add_head(&newcon->node, &console_list); + else + hlist_add_behind(&newcon->node, console_list.first); + + /* Ensure this flag is always set for the head of the list */ + cons_first()->flags |= CON_CONSDEV; newcon->dropped = 0; if (newcon->flags & CON_PRINTBUFFER) { @@ -3251,7 +3263,9 @@ void register_console(struct console *newcon) if (bootcon_enabled && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV) && !keep_bootcon) { - for_each_console(con) { + struct hlist_node *tmp; + + hlist_for_each_entry_safe(con, tmp, &console_list, node) { if (con->flags & CON_BOOT) console_unregister_locked(con); } @@ -3263,7 +3277,6 @@ EXPORT_SYMBOL(register_console); static int console_unregister_locked(struct console *console) { - struct console *con; int res; con_printk(KERN_INFO, console, "disabled\n"); @@ -3274,32 +3287,28 @@ static int console_unregister_locked(struct console *console) if (res > 0) return 0; - res = -ENODEV; console_lock(); - if (console_drivers == console) { - console_drivers=console->next; - res = 0; - } else { - for_each_console(con) { - if (con->next == console) { - con->next = console->next; - res = 0; - break; - } - } - } - if (res) - goto out_disable_unlock; + /* Disable it unconditionally */ + console->flags &= ~CON_ENABLED; + + if (hlist_unhashed(&console->node)) + goto out_unlock; + + hlist_del_init(&console->node); /* + * <HISTORICAL> * If this isn't the last console and it has CON_CONSDEV set, we * need to set it on the next preferred console. + * </HISTORICAL> + * + * The above makes no sense as there is no guarantee that the next + * console has any device attached. Oh well.... */ - if (console_drivers != NULL && console->flags & CON_CONSDEV) - console_drivers->flags |= CON_CONSDEV; + if (!hlist_empty(&console_list) && console->flags & CON_CONSDEV) + cons_first()->flags |= CON_CONSDEV; - console->flags &= ~CON_ENABLED; console_unlock(); console_sysfs_notify(); @@ -3308,10 +3317,8 @@ static int console_unregister_locked(struct console *console) return res; -out_disable_unlock: - console->flags &= ~CON_ENABLED; +out_unlock: console_unlock(); - return res; }