lkml.org 
[lkml]   [2022]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH printk v5 35/40] tty: serial: kgdboc: use srcu console list iterator
Date
Hi Doug,

On 2022-11-16, Doug Anderson <dianders@chromium.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@chromium.org>

Thanks.

John

\
 
 \ /
  Last update: 2022-11-17 10:33    [W:0.666 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site