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@xxxxxxxxxxxxx>
> ---
> 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
