Messages in this thread | | | Date | Fri, 11 Nov 2022 11:27:25 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH printk v3 39/40] printk: relieve console_lock of list synchronization duties |
| |
On Mon 2022-11-07 17:36:48, John Ogness wrote: > On 2022-11-07, John Ogness <john.ogness@linutronix.de> wrote: > > @@ -3344,7 +3340,6 @@ void register_console(struct console *newcon) > > * Put this console in the list - keep the > > * preferred driver at the head of the list. > > */ > > - console_lock(); > > if (hlist_empty(&console_list)) { > > /* Ensure CON_CONSDEV is always set for the head. */ > > newcon->flags |= CON_CONSDEV; > > @@ -3358,7 +3353,6 @@ void register_console(struct console *newcon) > > } else { > > hlist_add_behind_rcu(&newcon->node, console_list.first); > > } > > - console_unlock(); > > > > /* > > * No need to synchronize SRCU here! The caller does not rely > > I just realized that because of the new @seq initialization (patch 5/40) > that we cannot completely remove the console_lock from > register_console(). It will still be needed for @seq synchronization > when registering non-boot/non-printbuffer consoles. So something like > the patch below will need to be folded into this one.
Great catch!
> I am not happy with this. If an enabled boot console is behind, the > console_unlock() will probably catch it up and we will end up with some > repeat messages. But maybe this is "good enough" until we implement some > real coordination between boot console and normal console takeovers.
The same problem actually has been there even before. The new console was added in console_list under console_lock(). console_unlock() was called before the early consoles were unregistered.
A solution would be to call pr_flush() before. But it should be done in a separate patch.
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 17765166ac42..bb119001df56 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -3328,12 +3328,21 @@ void register_console(struct console *newcon) > * that message instead. That boot console will be > * unregistered shortly and may be the same device. > */ > + > + /* > + * Hold the console_lock to guarantee safe access to > + * console->seq. > + */ > + console_lock(); > + > for_each_console(con) { > if ((con->flags & (CON_BOOT | CON_ENABLED)) == (CON_BOOT | CON_ENABLED) && > con->seq < newcon->seq) { > newcon->seq = con->seq; > } > } > + > + console_unlock(); > }
This should be added already into the 5th patch that added this cycle. We just must keep it in this patch.
Best Regards, Petr
| |