Messages in this thread | | | From | John Ogness <> | Subject | Re: [PATCH printk v5 35/40] tty: serial: kgdboc: use srcu console list iterator | Date | Thu, 17 Nov 2022 10:38:07 +0106 |
| |
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
| |