Re: [PATCH v11 4/6] arm64: smp: Add arch support for backtrace using pseudo-NMI

Hi,

On Fri, Aug 25, 2023 at 3:27 PM Stephen Boyd [off-list ref] wrote:

Quoting Douglas Anderson (2023-08-24 08:30:30)

quoted

diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index fac08e18bcd5..50ce8b697ff3 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h

@@ -6,6 +6,9 @@

 #include <asm-generic/irq.h>

+void arch_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu);

Some nits, but otherwise

Reviewed-by: Stephen Boyd <redacted>

quoted

+#define arch_trigger_cpumask_backtrace arch_trigger_cpumask_backtrace
+
 struct pt_regs;

 int set_handle_irq(void (*handle_irq)(struct pt_regs *));

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index a5848f1ef817..c8896cbc5327 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c

@@ -72,12 +73,18 @@ enum ipi_msg_type {
        IPI_CPU_CRASH_STOP,
        IPI_TIMER,
        IPI_IRQ_WORK,
-       NR_IPI
+       NR_IPI,
+       /*
+        * Any enum >= NR_IPI and < MAX_IPI is special and not tracable
+        * with trace_ipi_*
+        */
+       IPI_CPU_BACKTRACE = NR_IPI,
+       MAX_IPI
 };

 static int ipi_irq_base __read_mostly;
 static int nr_ipi __read_mostly = NR_IPI;
-static struct irq_desc *ipi_desc[NR_IPI] __read_mostly;
+static struct irq_desc *ipi_desc[MAX_IPI] __read_mostly;

Side note: it would be nice to mark ipi_desc as __ro_after_init. Same
for nr_ipi and ipi_irq_base.

I'd rather not change it in this patch since it's a pre-existing and
separate issue, but I can add a patch to the end of the series for
that if I end up spinning it. Otherwise I can send a follow-up patch
for it.

quoted

 static void ipi_setup(int cpu);

@@ -845,6 +852,22 @@ static void __noreturn ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs
 #endif
 }

+static void arm64_backtrace_ipi(cpumask_t *mask)
+{
+       __ipi_send_mask(ipi_desc[IPI_CPU_BACKTRACE], mask);
+}
+
+void arch_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu)

Can this be 'bool exclude_self' instead of int? That matches all other
implementations from what I can tell.

Nope. See the part of the commit message that says:

This patch depends on commit 36759e343ff9 ("nmi_backtrace: allow
excluding an arbitrary CPU") since that commit changed the prototype
of arch_trigger_cpumask_backtrace(), which this patch implements.

quoted

+{
+       /*
+        * NOTE: though nmi_trigger_cpumask_backtrace has "nmi_" in the name,

USe nmi_trigger_cpumask_backtrace() to indicate function.

I won't plan on doing an immediate spin for this and for now I'll wait
for additional feedback. If a maintainer is getting ready to land
this, I'm happy to post a new version with this fix or also happy if a
maintainer wants to add the "()" while applying.

-Doug

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel