[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87tu2yncyw.fsf@jogness.linutronix.de>
Date: Thu, 17 Nov 2022 10:38:07 +0106
From: John Ogness <john.ogness@...utronix.de>
To: Doug Anderson <dianders@...omium.org>
Cc: Petr Mladek <pmladek@...e.com>,
Sergey Senozhatsky <senozhatsky@...omium.org>,
Steven Rostedt <rostedt@...dmis.org>,
Thomas Gleixner <tglx@...utronix.de>,
linux-kernel@...r.kernel.org,
Jason Wessel <jason.wessel@...driver.com>,
Daniel Thompson <daniel.thompson@...aro.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jirislaby@...nel.org>,
kgdb-bugreport@...ts.sourceforge.net, linux-serial@...r.kernel.org
Subject: Re: [PATCH printk v5 35/40] tty: serial: kgdboc: use srcu console
list iterator
Hi Doug,
On 2022-11-16, Doug Anderson <dianders@...omium.org> wrote:
>> @@ -463,9 +464,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;
>
> Is there truly any guarantee that "con" will be NULL if
> for_each_console_srcu() finishes naturally (AKA without a "break"
> being executed)?
Right now it is true because @con becoming NULL is the exit criteria for
the loop.
> It looks as if currently this will be true but nothing in the comments
> of for_each_console_srcu() nor hlist_for_each_entry_srcu() (which it
> calls) guarantees this, right? It would be nice if that was
> documented, but I guess it's not a huge deal.
Yes, if it is frowned upon that the iterator is used outside the loop,
it would be nice if the for_each macros explicitly provided some hints
in their documentation.
> Also: wasn't there just some big issue about people using loop
> iteration variables after the loop finished?
>
> https://lwn.net/Articles/885941/
Thanks for referencing that article! Indeed if the macros are changed so
that the iterator is defined in the loop, then code like this will
break. But I would expect that making such macro changes will also
require updating the call sites to avoid unused variables outside the
loops. And then this code could receive the appropriate fixup.
I feel like if I add extra code to guarantee a NULL without relying on
the macro implementation, I'll get more resistance due to unnecessarily
adding code and variables. But I may be wrong.
> Reviewed-by: Douglas Anderson <dianders@...omium.org>
Thanks.
John
Powered by blists - more mailing lists