| Message ID | 20250606-printk-cleanup-part2-v1-1-f427c743dda0@suse.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | printk cleanup - part 2 | expand |
On Fri 2025-06-06 23:53:43, Marcos Paulo de Souza wrote: > Since commit 9e70a5e109a4 ("printk: Add per-console suspended state") the > CON_SUSPENDED flag was introced, and this flag was being checked on > console_is_usable function, which returns false if the console is suspended. > > No functional changes. I double checked potential functional changes. In particular, I checked where the CON_ENABLED and CON_SUSPENDED flags were used. Both flags seems to have the same effect in most situations, for example, in console_is_usable() or console_unblank(). But there seems to be two exceptions: kdb_msg_write() and show_cons_active(). These two functions check only the CON_ENABLED flag. And they think that the console is usable when the flag is set. The change in this patch would change the behavior of the two functions during suspend. It is later fixed by the 3rd and 4th patch. But it might cause regressions during bisections. It is probably not a big deal because the system is not much usable during the suspend anyway. But still, I would feel more comfortable if we prevented the "temporary" regression. I see two possibilities: 1. Merge the 3rd and 4th patch into this one. It would change the semantic in a single patch. 2. First update kdb_msg_write() and show_cons_active() to check both CON_ENABLE and CON_SUSPENDED flags. The 1st solution probably makes more sense because we are going to remove the CON_ENABLE flag in the end. And even the merged patch is small enough. Best Regards, Petr
On Thu, 2025-06-12 at 13:46 +0200, Petr Mladek wrote: > On Fri 2025-06-06 23:53:43, Marcos Paulo de Souza wrote: > > Since commit 9e70a5e109a4 ("printk: Add per-console suspended > > state") the > > CON_SUSPENDED flag was introced, and this flag was being checked on > > console_is_usable function, which returns false if the console is > > suspended. > > > > No functional changes. > > I double checked potential functional changes. In particular, I > checked where the CON_ENABLED and CON_SUSPENDED flags were used. > > Both flags seems to have the same effect in most situations, > for example, in console_is_usable() or console_unblank(). > > But there seems to be two exceptions: kdb_msg_write() and > show_cons_active(). These two functions check only > the CON_ENABLED flag. And they think that the console is > usable when the flag is set. > > The change in this patch would change the behavior of the two > functions during suspend. It is later fixed by the 3rd and 4th > patch. But it might cause regressions during bisections. > > It is probably not a big deal because the system is not much > usable during the suspend anyway. But still, I would feel more > comfortable if we prevented the "temporary" regression. > > I see two possibilities: > > 1. Merge the 3rd and 4th patch into this one. It would change > the semantic in a single patch. I liked the suggestion, I'll do it in the next revision. Thanks for looking into it! > > 2. First update kdb_msg_write() and show_cons_active() > to check both CON_ENABLE and CON_SUSPENDED flags. > > The 1st solution probably makes more sense because we are going > to remove the CON_ENABLE flag in the end. And even the merged > patch is small enough. > > Best Regards, > Petr
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 1eea80d0648ed3583375cce3dfe60407894d659c..6d3cf488f4261a3dfd8809a5ab7164b218238c13 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3530,7 +3530,7 @@ void console_suspend(struct console *console) { __pr_flush(console, 1000, true); console_list_lock(); - console_srcu_write_flags(console, console->flags & ~CON_ENABLED); + console_srcu_write_flags(console, console->flags | CON_SUSPENDED); console_list_unlock(); /* @@ -3543,13 +3543,14 @@ void console_suspend(struct console *console) } EXPORT_SYMBOL(console_suspend); +/* Unset CON_SUSPENDED flag so the console can start printing again. */ void console_resume(struct console *console) { struct console_flush_type ft; bool is_nbcon; console_list_lock(); - console_srcu_write_flags(console, console->flags | CON_ENABLED); + console_srcu_write_flags(console, console->flags & ~CON_SUSPENDED); is_nbcon = console->flags & CON_NBCON; console_list_unlock();
Since commit 9e70a5e109a4 ("printk: Add per-console suspended state") the CON_SUSPENDED flag was introced, and this flag was being checked on console_is_usable function, which returns false if the console is suspended. No functional changes. Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- kernel/printk/printk.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)