linux-kernel - Re: [PATCH printk v5 15/17] printk: Implement legacy printer kthread for PREEMPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874j6wke6e.fsf@jogness.linutronix.de>
Date: Tue, 03 Sep 2024 16:30:57 +0206
From: John Ogness <john.ogness@...utronix.de>
To: Petr Mladek <pmladek@...e.com>
Cc: Sergey Senozhatsky <senozhatsky@...omium.org>, Steven Rostedt
 <rostedt@...dmis.org>, Thomas Gleixner <tglx@...utronix.de>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH printk v5 15/17] printk: Implement legacy printer
 kthread for PREEMPT_RT

On 2024-09-03, Petr Mladek <pmladek@...e.com> wrote:
>> +static bool legacy_kthread_should_wakeup(void)
>> +{
>> +	struct console_flush_type ft;
>> +	struct console *con;
>> +	bool ret = false;
>> +	int cookie;
>> +
>> +	if (kthread_should_stop())
>> +		return true;
>> +
>> +	printk_get_console_flush_type(&ft);
>> +
>> +	cookie = console_srcu_read_lock();
>> +	for_each_console_srcu(con) {
>> +		short flags = console_srcu_read_flags(con);
>> +		u64 printk_seq;
>> +
>> +		/*
>> +		 * The legacy printer thread is only for legacy consoles when
>> +		 * the nbcon consoles have their printer threads.
>> +		 */
>> +		if ((flags & CON_NBCON) && ft.nbcon_offload)
>> +			continue;
>
> I am still scratching my head about the fact that the legacy loop
> probably should not handle the nbcon consoles also when
> printk_get_console_flush_type() returns ft.nbcon_atomic().

The legacy loop kthread should never handle nbcon consoles unless a boot
console is registered. So it really needs to be:

if ((flags & CON_NBCON) && (ft.nbcon_offload || ft.nbcon_atomic))

> We probably does not have to take care of it here because this
> code is called only when the legacy kthread is running.
> It means that nbcon consoles should have their kthreads as well
> when they can be handled outside the legacy loop. I mean
> that we should never see ft.nbcon_atomic set here.

Remember that the nbcon kthreads are stopped on shutdown/reboot. For
those final messages, ft.nbcon_atomic=1. The legacy loop kthread should
not handled those printings.

> Sigh, the logic is so complicated.

It takes some getting used to. But I feel like the responsibilities and
transitions are clearer now than ever before.

>> --- a/kernel/printk/printk_safe.c
>> +++ b/kernel/printk/printk_safe.c
>> @@ -44,7 +44,9 @@ bool is_printk_legacy_deferred(void)
>>  	 * The per-CPU variable @printk_context can be read safely in any
>>  	 * context. CPU migration is always disabled when set.
>>  	 */
>> -	return (this_cpu_read(printk_context) || in_nmi());
>> +	return (force_legacy_kthread() ||
>
> This is not correct when used in panic(). force_legacy_kthread()
> is not a reason for offload in that case.

If force_legacy_kthread() is true, legacy printing is _ALWAYS_ offloaded
except as a "hope and pray" last attempt via
console_flush_on_panic(). That is a requirement of PREEMPT_RT.

John