Hi Michael,
Michael Ellerman <m...@ellerman.id.au> writes:
> Nathan Lynch <nath...@linux.ibm.com> writes:
>> When smp_send_safe_nmi_ipi() indicates that the target CPU has
>> responded to the IPI, skip the remote paca inspection
>> fallback. Otherwise both the sending and target CPUs attempt the
>> backtrace, usually creating a misleading ("didn't respond to backtrace
>> IPI" is wrong) and interleaved mess:
>
> Thanks for fixing my bugs for me :)
>
Thanks for your review! I was beginning to think I had missed some
subtletly here, thanks for illustrating it.
I'll run with your proposed change below for the problem I'm working.
> To solve it I think we want to avoid clearing a CPU from the mask unless
> we know that the IPI failed for that CPU. That way there's no risk of
> suppressing a trace from a CPU that successfully handles the IPI, and we
> know we've waited 5 seconds for CPUs that fail to handle the IPI.
>
> I don't think we want to allocate a whole new cpumask to track which
> CPUs have failed to respond, but I don't think we need to. We can just
> synchronously handle them.
>
> diff --git a/arch/powerpc/kernel/stacktrace.c
> b/arch/powerpc/kernel/stacktrace.c
> index 1deb1bf331dd..980e87f7ae7a 100644
> --- a/arch/powerpc/kernel/stacktrace.c
> +++ b/arch/powerpc/kernel/stacktrace.c
> @@ -172,17 +172,19 @@ static void handle_backtrace_ipi(struct pt_regs *regs)
>
> static void raise_backtrace_ipi(cpumask_t *mask)
> {
> + struct paca_struct *p;
> unsigned int cpu;
>
> for_each_cpu(cpu, mask) {
> - if (cpu == smp_processor_id())
> + if (cpu == smp_processor_id()) {
> handle_backtrace_ipi(NULL);
> - else
> - smp_send_safe_nmi_ipi(cpu, handle_backtrace_ipi, 5 *
> USEC_PER_SEC);
> - }
> + continue;
> + }
>
> - for_each_cpu(cpu, mask) {
> - struct paca_struct *p = paca_ptrs[cpu];
> + if (smp_send_safe_nmi_ipi(cpu, handle_backtrace_ipi, 5 *
> USEC_PER_SEC))
> + continue;
> +
> + p = paca_ptrs[cpu];
>
> cpumask_clear_cpu(cpu, mask);
>
