linux-kernel - Re: [PATCH v3 2/2] Handle flushing of CPU backtraces during panic

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: <20240821050254.69853-1-takakura@valinux.co.jp>
Date: Wed, 21 Aug 2024 14:02:54 +0900
From: takakura@...inux.co.jp
To: pmladek@...e.com,
	john.ogness@...utronix.de
Cc: akpm@...ux-foundation.org,
	bhe@...hat.com,
	feng.tang@...el.com,
	j.granados@...sung.com,
	linux-kernel@...r.kernel.org,
	lukas@...ner.de,
	nishimura@...inux.co.jp,
	rostedt@...dmis.org,
	senozhatsky@...omium.org,
	stephen.s.brennan@...cle.com,
	taka@...inux.co.jp,
	takakura@...inux.co.jp,
	ubizjak@...il.com,
	wangkefeng.wang@...wei.com
Subject: Re: [PATCH v3 2/2] Handle flushing of CPU backtraces during panic

Hi Petr and John,

On 2024-08-19, John Ogness wrote:
>On 2024-08-13, Petr Mladek <pmladek@...e.com> wrote:
>> I would do something like:
>>
>> /**
>>  * console_try_or_trigger_flush - try to flush consoles directly when
>>  *   safe or the trigger deferred flush.
>>  *
>>  * Context: Any
>>  */
>> void console_try_or_trigger_flush(void)
>> {
>>      if (!is_printk_legacy_deferred() && console_trylock())
>>              console_unlock();
>>      else
>>              defer_console_output();
>> }
>>
>> and use it instead of printk_trigger_flush() in
>> nmi_trigger_cpumask_backtrace().
>
>Just to be clear, you are talking about removing printk_trigger_flush()
>entirely and instead provide the new console_try_or_trigger_flush()?
>Which then also involves updating the call sites:
>
>lib/nmi_backtrace.c:nmi_trigger_cpumask_backtrace()
>arch/powerpc/kernel/watchdog.c:watchdog_timer_interrupt()
>

Taking a look at [0], in addition to the mentioned call sites, 
nbcon_device_release() will also be calling printk_trigger_flush()?
For nbcon_device_release(), I thought its better not to be replaced as 
it calles for @legacy_off, in which case printk_trigger_flush() seems 
more suitable as it always defers printing.

Also taking a look at the [1], for nmi_trigger_cpumask_backtrace(), 
I thought that it will not comply with the syncing of 
legacy_allow_panic_sync. I believe it will allow flushing of legacy consoles 
before printk_legacy_allow_panic_sync() which is out of sync.

>> Well, I would postpone this patch after we finalize the patchset
>> adding con->write_atomic() callback. This patch depends on it anyway
>> via is_printk_legacy_deferred(). The patchset might also add
>> other wrappers for flushing consoles and we have to choose some
>> reasonable names.
>
>I agree. Let's finish up the atomic series and then we can worry about
>this.
>

Ok! I see that it can be better discussed after the atomic series.

>John

Sincerely,
Ryo Takakura

[0] https://lore.kernel.org/all/20240820063001.36405-31-john.ogness@linutronix.de/
[1] https://lore.kernel.org/all/20240820063001.36405-30-john.ogness@linutronix.de/