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.ogn...@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


_______________________________________________
Kgdb-bugreport mailing list
Kgdb-bugreport@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

Reply via email to