[PATCH] powerpc/crash: Allow direct printing on kexec

[PATCH] powerpc/crash: Allow direct printing on kexec

[Ryo Takakura]

Since the commit af2876b501e4 ("powerpc/crash: Use NMI context for printk
when starting to crash"), printing has been deferred before shutting down
non-panicked CPU on kexec to avoid deadlock on logbuf_lock. It is deferred
until the shutdown of the first kernel and starts booting into the second
kernel. As a result, there is no messages printed for legacy consoles,
including crash_kexec_post_notifiers messages which is after the
syncing of legacy console at printk_legacy_allow_panic_sync().

Let legacy consoles print directly so that we can see messages on kexec, as
the commit b6cf8b3f3312 ("printk: add lockless ringbuffer") turned printk
ring buffer lockless and there should be no worries panicked CPU 
deadlocking writing into ringbuffer after shutting down non-panicked CPU.

Signed-off-by: Ryo Takakura <ryotkkr98@gmail.com>
---

Hi!

My understanding is that deferred printing can also be safely removed 
in terms of console lock as the commit d51507098ff9 ("printk: disable 
optimistic spin during panic") prevented from spinning in case of panic.

Sincerely,
Ryo Takakura

---
 arch/powerpc/kexec/crash.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index 9ac3266e4965..5e5260e0d964 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -269,9 +269,6 @@ static inline void crash_kexec_wait_realmode(int cpu) {}
 
 void crash_kexec_prepare(void)
 {
-	/* Avoid hardlocking with irresponsive CPU holding logbuf_lock */
-	printk_deferred_enter();
-
 	/*
 	 * This function is only called after the system
 	 * has panicked or is otherwise in a critical state.
-- 
2.34.1

Re: [PATCH] powerpc/crash: Allow direct printing on kexec

[John Ogness]

Hi Ryo,

On 2024-10-20, Ryo Takakura <ryotkkr98@gmail.com> wrote:
> Let legacy consoles print directly so that we can see messages on kexec, as
> the commit b6cf8b3f3312 ("printk: add lockless ringbuffer") turned printk
> ring buffer lockless and there should be no worries panicked CPU 
> deadlocking writing into ringbuffer after shutting down non-panicked CPU.

It is correct that the ringbuffer is now lockless. But the legacy
console drivers are not. Allowing them to print directly in panic can
lead to similar effects that commit af2876b501e4 ("powerpc/crash: Use
NMI context for printk when starting to crash") was working around.

Note that although printk is deferred, it is only the printing that is
deferred. The messages are landing in the ringbuffer immediately. So
they would be available to kdump and crash kernels.

Rather than removing the deferring, it would be better to convert the
console you are using to the new NBCON API. Then it would be able to
print direct and safe during panic. (printk_deferred does not affect
NBCON consoles.) What console driver are you using that you want to see
the messages on?

John Ogness

Re: [PATCH] powerpc/crash: Allow direct printing on kexec

[Ryo Takakura]

Hi John!

On 2024-10-20, John Ogness <john.ogness@linutronix.de> wrote:
>On 2024-10-20, Ryo Takakura <ryotkkr98@gmail.com> wrote:
>> Let legacy consoles print directly so that we can see messages on kexec, as
>> the commit b6cf8b3f3312 ("printk: add lockless ringbuffer") turned printk
>> ring buffer lockless and there should be no worries panicked CPU 
>> deadlocking writing into ringbuffer after shutting down non-panicked CPU.
>
>It is correct that the ringbuffer is now lockless. But the legacy
>console drivers are not. Allowing them to print directly in panic can
>lead to similar effects that commit af2876b501e4 ("powerpc/crash: Use
>NMI context for printk when starting to crash") was working around.

Oh I see.
I wasn't taking the locks acquired by console drivers into account...
Thanks once again for the feedback!

>Note that although printk is deferred, it is only the printing that is
>deferred. The messages are landing in the ringbuffer immediately. So
>they would be available to kdump and crash kernels.

I also agree to rather skip printing and leave it to kdump and 
crash kernels considering the chance of pointed out risk on panic.

>Rather than removing the deferring, it would be better to convert the
>console you are using to the new NBCON API. Then it would be able to
>print direct and safe during panic. (printk_deferred does not affect
>NBCON consoles.) What console driver are you using that you want to see
>the messages on?

I was working on qemu ppc64 this time but I am usually working on 
Raspberry Pi 4 (mostly for fun and study) which uses either of 
bcm2835-aux-uart or amba-pl011. It would be really nice to see them 
working as nbcon!
I am thinking of taking a look at [0] but If there were any other 
references, I would really like to look into as well.

>John Ogness

Sincerely,
Ryo Takakura

[0] https://lore.kernel.org/lkml/87wn3zsz5x.fsf@jogness.linutronix.de/

Re: [PATCH] powerpc/crash: Allow direct printing on kexec

[John Ogness]

On 2024-10-21, Ryo Takakura <ryotkkr98@gmail.com> wrote:
>> Rather than removing the deferring, it would be better to convert the
>> console you are using to the new NBCON API. Then it would be able to
>> print direct and safe during panic. (printk_deferred does not affect
>> NBCON consoles.) What console driver are you using that you want to
>> see the messages on?
>
> I was working on qemu ppc64 this time but I am usually working on 
> Raspberry Pi 4 (mostly for fun and study) which uses either of 
> bcm2835-aux-uart or amba-pl011. It would be really nice to see them 
> working as nbcon!
> I am thinking of taking a look at [0] but If there were any other 
> references, I would really like to look into as well.
>
> [0] https://lore.kernel.org/lkml/87wn3zsz5x.fsf@jogness.linutronix.de/

The lastest version of the series is here [1]. The series is still
undergoing revisions, however the changes are 8250-specific. The API for
nbcon consoles is already available since 6.12-rc1. You are certainly
welcome to try to convert one of those Raspi 4 drivers to the nbcon
interface. I would be happy to review it.

John Ogness

[1] https://lore.kernel.org/lkml/20240913140538.221708-1-john.ogness@linutronix.de

Re: [PATCH] powerpc/crash: Allow direct printing on kexec

[Ryo Takakura]

On 2024-10-21, John Ogness <john.ogness@linutronix.de> wrote:
>On 2024-10-21, Ryo Takakura <ryotkkr98@gmail.com> wrote:
>>> Rather than removing the deferring, it would be better to convert the
>>> console you are using to the new NBCON API. Then it would be able to
>>> print direct and safe during panic. (printk_deferred does not affect
>>> NBCON consoles.) What console driver are you using that you want to
>>> see the messages on?
>>
>> I was working on qemu ppc64 this time but I am usually working on 
>> Raspberry Pi 4 (mostly for fun and study) which uses either of 
>> bcm2835-aux-uart or amba-pl011. It would be really nice to see them 
>> working as nbcon!
>> I am thinking of taking a look at [0] but If there were any other 
>> references, I would really like to look into as well.
>>
>> [0] https://lore.kernel.org/lkml/87wn3zsz5x.fsf@jogness.linutronix.de/
>
>The lastest version of the series is here [1]. The series is still
>undergoing revisions, however the changes are 8250-specific. The API for
>nbcon consoles is already available since 6.12-rc1. You are certainly
>welcome to try to convert one of those Raspi 4 drivers to the nbcon
>interface. I would be happy to review it.

Thanks for sharing!
I will take a look at it and definitley give a try!

>John Ogness

Sincerely,
Ryo Takakura