Re: [PATCH printk v2 4/5] printk: Add per-console suspended state
From: John Ogness
Date: Tue Jul 11 2023 - 11:23:51 EST
On 2023-07-12, Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> wrote:
>> @@ -2623,8 +2647,6 @@ void console_lock(void)
>> msleep(1000);
>>
>> down_console_sem();
>> - if (console_suspended)
>> - return;
>> console_locked = 1;
>> console_may_schedule = 1;
>> }
>> @@ -2645,10 +2667,6 @@ int console_trylock(void)
>> return 0;
>> if (down_trylock_console_sem())
>> return 0;
>> - if (console_suspended) {
>> - up_console_sem();
>> - return 0;
>> - }
>> console_locked = 1;
>> console_may_schedule = 0;
>> return 1;
>
> Interesting. console_locked previously would not be set if
> console is suspended, but now it's always set, which in theory
> changes the way WARN_CONSOLE_UNLOCKED() macro works in some
> cases?
Yes, Petr mentioned [0] this during the v1 review. His direct comment:
"console_locked" seems to be used only in WARN_CONSOLE_UNLOCKED().
I could imagine a corner case where, for example, "vt" code does
not print the warning because it works as it works. But it does
not make much sense. IMHO, such a code should get fixed. And it
is just a warning after all.
And his final comment in that thread:
I believe that @console_suspended is not longer needed.
Let's replace it with the per-console flag and do not worry
about races.
John Ogness
[0] https://lore.kernel.org/lkml/ZAieQtcs7YEuCCDa@alley