Currently, if printk lock (logbuf_lock) is held by other thread during
crash, there is a chance of deadlocking the crash on next printk, and
blocking a possibly desired kdump.
After sending IPI to all other CPUs, make printk enter in NMI context,
as it will use per-cpu buffers to store the message, and avoid locking
logbuf_lock.
Signed-off-by: Leonardo Bras <[email protected]>
---
arch/powerpc/kexec/crash.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index d488311efab1..9b73e3991bf4 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -115,6 +115,7 @@ static void crash_kexec_prepare_cpus(int cpu)
crash_send_ipi(crash_ipi_callback);
smp_wmb();
+ printk_nmi_enter();
again:
/*
--
2.25.1
Leonardo Bras <[email protected]> writes:
> Currently, if printk lock (logbuf_lock) is held by other thread during
> crash, there is a chance of deadlocking the crash on next printk, and
> blocking a possibly desired kdump.
>
> After sending IPI to all other CPUs, make printk enter in NMI context,
> as it will use per-cpu buffers to store the message, and avoid locking
> logbuf_lock.
>
> Signed-off-by: Leonardo Bras <[email protected]>
> ---
> arch/powerpc/kexec/crash.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
> index d488311efab1..9b73e3991bf4 100644
> --- a/arch/powerpc/kexec/crash.c
> +++ b/arch/powerpc/kexec/crash.c
> @@ -115,6 +115,7 @@ static void crash_kexec_prepare_cpus(int cpu)
Added context:
printk(KERN_EMERG "Sending IPI to other CPUs\n");
if (crash_wake_offline)
ncpus = num_present_cpus() - 1;
>
> crash_send_ipi(crash_ipi_callback);
> smp_wmb();
> + printk_nmi_enter();
Why did you decide to put it there, rather than at the start of
default_machine_crash_shutdown() like I did?
The printk() above could have already deadlocked if another CPU is stuck
with the logbuf lock held.
cheers
On Wed, 2020-04-08 at 22:13 +1000, Michael Ellerman wrote:
[...]
> Added context:
>
> printk(KERN_EMERG "Sending IPI to other CPUs\n");
>
> if (crash_wake_offline)
> ncpus = num_present_cpus() - 1;
>
> >
> > crash_send_ipi(crash_ipi_callback);
> > smp_wmb();
> > + printk_nmi_enter();
>
> Why did you decide to put it there, rather than at the start of
> default_machine_crash_shutdown() like I did?
>
> The printk() above could have already deadlocked if another CPU is stuck
> with the logbuf lock held.
Oh, I thought the CPUs would start crashing after crash_send_ipi(), so
only printk() after that would possibly deadlock.
I was not able to see how the printk() above would deadlock, but I see
no problem adding that at the start of the function.
Best regards,
Leonardo Bras