| Message ID | 20221019145600.1282823-22-john.ogness@linutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | [printk,v2,01/38] serial: kgdboc: Lock console list in probe function | expand |
On Wed 2022-10-19 17:01:43, John Ogness wrote: > Use srcu console list iteration for safe console list traversal. > > Note that configure_kgdboc() still requires the console_lock in > order to ensure that no console is in its write() callback when > its direct() callback is called. Add comments to clarify this. s/direct()/device()/ Do you know about such requirements or is it just a conservative approach, please? I ask because the comment in the code says "may assume". Anyway, this would deserve a comment why the SRCU list iteration is needed even when console_lock() is needed as well. The reason is that further patches are going to synchronize console_list manipulation with another lock and console_lock() will be used only to serialize accessing con->write() callbacks. Best Regards, Petr > Signed-off-by: John Ogness <john.ogness@linutronix.de> > --- > drivers/tty/serial/kgdboc.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c > index b17aa7e49894..e9d3f8c6e3dc 100644 > --- a/drivers/tty/serial/kgdboc.c > +++ b/drivers/tty/serial/kgdboc.c > @@ -171,6 +171,7 @@ static int configure_kgdboc(void) > int err = -ENODEV; > char *cptr = config; > struct console *cons; > + int cookie; > > if (!strlen(config) || isspace(config[0])) { > err = 0; > @@ -193,8 +194,14 @@ static int configure_kgdboc(void) > if (!p) > goto noconfig; > > + /* > + * Stop console printing because the device() callback may > + * assume the console is not within its write() callback. > + */ > console_lock(); > - for_each_console(cons) { > + > + cookie = console_srcu_read_lock(); > + for_each_console_srcu(cons) { > int idx; > if (cons->device && cons->device(cons, &idx) == p && > idx == tty_line) { > @@ -202,6 +209,8 @@ static int configure_kgdboc(void) > break; > } > } > + console_srcu_read_unlock(cookie); > + > console_unlock(); > > kgdb_tty_driver = p; > @@ -451,6 +460,7 @@ static void kgdboc_earlycon_pre_exp_handler(void) > { > struct console *con; > static bool already_warned; > + int cookie; > > if (already_warned) > return; > @@ -463,9 +473,14 @@ static void kgdboc_earlycon_pre_exp_handler(void) > * serial drivers might be OK with this, print a warning once per > * boot if we detect this case. > */ > - for_each_console(con) > + cookie = console_srcu_read_lock(); > + for_each_console_srcu(con) { > if (con == kgdboc_earlycon_io_ops.cons) > - return; > + break; > + } > + console_srcu_read_unlock(cookie); > + if (con) > + return; > > already_warned = true; > pr_warn("kgdboc_earlycon is still using bootconsole\n"); > -- > 2.30.2
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c index b17aa7e49894..e9d3f8c6e3dc 100644 --- a/drivers/tty/serial/kgdboc.c +++ b/drivers/tty/serial/kgdboc.c @@ -171,6 +171,7 @@ static int configure_kgdboc(void) int err = -ENODEV; char *cptr = config; struct console *cons; + int cookie; if (!strlen(config) || isspace(config[0])) { err = 0; @@ -193,8 +194,14 @@ static int configure_kgdboc(void) if (!p) goto noconfig; + /* + * Stop console printing because the device() callback may + * assume the console is not within its write() callback. + */ console_lock(); - for_each_console(cons) { + + cookie = console_srcu_read_lock(); + for_each_console_srcu(cons) { int idx; if (cons->device && cons->device(cons, &idx) == p && idx == tty_line) { @@ -202,6 +209,8 @@ static int configure_kgdboc(void) break; } } + console_srcu_read_unlock(cookie); + console_unlock(); kgdb_tty_driver = p; @@ -451,6 +460,7 @@ static void kgdboc_earlycon_pre_exp_handler(void) { struct console *con; static bool already_warned; + int cookie; if (already_warned) return; @@ -463,9 +473,14 @@ static void kgdboc_earlycon_pre_exp_handler(void) * serial drivers might be OK with this, print a warning once per * boot if we detect this case. */ - for_each_console(con) + cookie = console_srcu_read_lock(); + for_each_console_srcu(con) { if (con == kgdboc_earlycon_io_ops.cons) - return; + break; + } + console_srcu_read_unlock(cookie); + if (con) + return; already_warned = true; pr_warn("kgdboc_earlycon is still using bootconsole\n");
Use srcu console list iteration for safe console list traversal. Note that configure_kgdboc() still requires the console_lock in order to ensure that no console is in its write() callback when its direct() callback is called. Add comments to clarify this. Signed-off-by: John Ogness <john.ogness@linutronix.de> --- drivers/tty/serial/kgdboc.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)