| Message ID | 20250606-printk-cleanup-part2-v1-6-f427c743dda0@suse.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | printk cleanup - part 2 | expand |
On Fri 2025-06-06 23:53:48, Marcos Paulo de Souza wrote: > All consoles found on for_each_console_srcu are registered, meaning that all of > them are CON_ENABLED. The code tries to find an active console, so check if the > console is not suspended instead. > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > --- > kernel/debug/kdb/kdb_io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c > index 9b11b10b120cf07e451a7a4d92ce50f9a6c066b2..cdc1ee81d7332a9a00b967af719939f438f26cef 100644 > --- a/kernel/debug/kdb/kdb_io.c > +++ b/kernel/debug/kdb/kdb_io.c > @@ -589,7 +589,7 @@ static void kdb_msg_write(const char *msg, int msg_len) > */ > cookie = console_srcu_read_lock(); > for_each_console_srcu(c) { > - if (!(console_srcu_read_flags(c) & CON_ENABLED)) > + if (console_srcu_read_flags(c) & CON_SUSPENDED) > continue; I think that this is similar to the 5th patch. We should check here is_console_usable(con, console_srcu_read_flags(c), true) because it checks more conditions: + the global console_suspended flag. The consoles drivers should not be used when it is set... + whether NBCON console driver has con->write_atomic and we should also fix kdb_msg_write() to actually use con->write_atomic() when it is a NBCON console driver. There is hard-coded con->write() at the moment. But it might get more complicated. It would be nice to do it correctly and use con->write_atomit() only when nbcon_context_try_acquire() succeeds. We probably should use a context with NBCON_PRIO_EMERGENCY. And this should be fixed at the beginning of the patchset because it actually fixes the support of the new NBCON console drivers. Best Regards, Petr > if (c == dbg_io_ops->cons) > continue; > > -- > 2.49.0
On Mon, 2025-06-16 at 15:56 +0200, Petr Mladek wrote: > On Fri 2025-06-06 23:53:48, Marcos Paulo de Souza wrote: > > All consoles found on for_each_console_srcu are registered, meaning > > that all of > > them are CON_ENABLED. The code tries to find an active console, so > > check if the > > console is not suspended instead. > > > > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> > > --- > > kernel/debug/kdb/kdb_io.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c > > index > > 9b11b10b120cf07e451a7a4d92ce50f9a6c066b2..cdc1ee81d7332a9a00b967af7 > > 19939f438f26cef 100644 > > --- a/kernel/debug/kdb/kdb_io.c > > +++ b/kernel/debug/kdb/kdb_io.c > > @@ -589,7 +589,7 @@ static void kdb_msg_write(const char *msg, int > > msg_len) > > */ > > cookie = console_srcu_read_lock(); > > for_each_console_srcu(c) { > > - if (!(console_srcu_read_flags(c) & CON_ENABLED)) > > + if (console_srcu_read_flags(c) & CON_SUSPENDED) > > continue; > > I think that this is similar to the 5th patch. We should check > here is_console_usable(con, console_srcu_read_flags(c), true) > because it checks more conditions: > > + the global console_suspended flag. The consoles drivers should > not be used when it is set... > > + whether NBCON console driver has con->write_atomic > Makes sense, I'll work on it first then. > and we should also fix kdb_msg_write() to actually use > con->write_atomic() when it is a NBCON console driver. > There is hard-coded con->write() at the moment. > > But it might get more complicated. It would be nice to do it > correctly > and use con->write_atomit() only when nbcon_context_try_acquire() > succeeds. We probably should use a context with NBCON_PRIO_EMERGENCY. > I'll check how that can be done by looking at Docs. > And this should be fixed at the beginning of the patchset because > it actually fixes the support of the new NBCON console drivers. I'll try to send patches for this case first, and then come back with this series once this one is fixed, so I can start converting the other places like you suggested later. Thanks a lot for the suggestion! > > Best Regards, > Petr > > > if (c == dbg_io_ops->cons) > > continue; > > > > -- > > 2.49.0
diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c index 9b11b10b120cf07e451a7a4d92ce50f9a6c066b2..cdc1ee81d7332a9a00b967af719939f438f26cef 100644 --- a/kernel/debug/kdb/kdb_io.c +++ b/kernel/debug/kdb/kdb_io.c @@ -589,7 +589,7 @@ static void kdb_msg_write(const char *msg, int msg_len) */ cookie = console_srcu_read_lock(); for_each_console_srcu(c) { - if (!(console_srcu_read_flags(c) & CON_ENABLED)) + if (console_srcu_read_flags(c) & CON_SUSPENDED) continue; if (c == dbg_io_ops->cons) continue;
All consoles found on for_each_console_srcu are registered, meaning that all of them are CON_ENABLED. The code tries to find an active console, so check if the console is not suspended instead. Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com> --- kernel/debug/kdb/kdb_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)