Messages in this thread | | | Date | Tue, 15 Nov 2022 19:36:06 +0100 | From | Petr Mladek <> | Subject | Re: replay log: Re: [PATCH printk v4 38/39] printk: relieve console_lock of list synchronization duties |
| |
On Tue 2022-11-15 18:21:34, John Ogness wrote: > On 2022-11-15, Petr Mladek <pmladek@suse.com> wrote: > >>> --- a/kernel/printk/printk.c > >>> +++ b/kernel/printk/printk.c > >>> @@ -3334,6 +3330,11 @@ void register_console(struct console *newcon) > >>> * boot console that is the furthest behind. > >>> */ > >>> if (bootcon_registered && !keep_bootcon) { > >>> + /* > >>> + * Hold the console_lock to guarantee safe access to > >>> + * console->seq. > >>> + */ > >>> + console_lock(); > >>> for_each_console(con) { > >>> if ((con->flags & CON_BOOT) && > >>> (con->flags & CON_ENABLED) && > >>> @@ -3341,6 +3342,7 @@ void register_console(struct console *newcon) > >>> newcon->seq = con->seq; > >>> } > >>> } > >>> + console_unlock(); > > > > So, without the above two hunks: > > > > Reviewed-by: Petr Mladek <pmladek@suse.com> > > Note that we actually need those hunks to guarantee a consistent @seq > value. The console_lock is the only synchronization mechanism available > to read console->seq.
Yes, we need a solution. But it does not need to be in this patch.
This patch removes console_lock() on some locations. But this particular code was called without console_lock() even before this patch.
Note that the regression was added in the 3rd patch that moved this code outside console_lock().
Maybe, the easiest solution would be to do in the 3rd patch [*]:
} else { /* Begin with next message. */ newcon->seq = prb_next_seq(prb); /* * Try hard to show the pending messages on boot consoles. * so that the new console does not start too late. */ pr_flush(); }
It should behave as good and as bad as the original code.
[*] Or move the code and add this change before the 3rd patch to keep this questionable solution separated and avoid the regression.
Best Regards, Petr
| |