Messages in this thread Patch in this message | | | Date | Thu, 23 May 2019 15:59:47 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [PATCHv2 4/4] printk: make sure we always print console disabled message |
| |
On (05/15/19 16:47), Petr Mladek wrote: > On Fri 2019-04-26 14:44:45, Sergey Senozhatsky wrote: > > > > Forgot to mention that the series is still in RFC phase. > > > > > > On (04/26/19 14:33), Sergey Senozhatsky wrote: > > [..] > > > +++ b/kernel/printk/printk.c > > > @@ -2613,6 +2613,12 @@ static int __unregister_console(struct console *console) > > > pr_info("%sconsole [%s%d] disabled\n", > > > (console->flags & CON_BOOT) ? "boot" : "", > > > console->name, console->index); > > > + /* > > > + * Print 'console disabled' on all the consoles, including the > > > + * one we are about to unregister. > > > + */ > > > + console_unlock(); > > > + console_lock(); > > > > > > res = _braille_unregister_console(console); > > > if (res) > > > > Need to think more if this is race free... > > I am afraid that it is racy against for_each_console() when > removing the boot consoles.
Can you explain? Do you mean that we can execute two paths unregistering the same bcon?
CPU0 CPU1
console_lock(); __unregister_console(bconA) console_lock(); console_unlock();
__unregister_console(bconA); for (a = console_drivers->next ...) if (a == console) unregister(); // console bconA is // not in the list // anymore console_unlock();
for (a = console_drivers->next ...) if (a == console) console_unlock();
This CPU0 will never see bconA in the console drivers list. But... it will try to do
console->flags &= ~CON_ENABLED;
Which we need to fix.
Do not disable console which is not on the console_drivers list.
--- index 1177ea4b3fe1..e729992cb4e4 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2660,7 +2660,8 @@ static int __unregister_console(struct console *console) if (console_drivers != NULL && console->flags & CON_CONSDEV) console_drivers->flags |= CON_CONSDEV; - console->flags &= ~CON_ENABLED; + if (!res) + console->flags &= ~CON_ENABLED; return res; } --- -ss
| |