[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD=FV=U24kF7WbZPKMbk=kJMU14+yFJXpGWWfj0fPbA9J6h0Eg@mail.gmail.com>
Date: Fri, 25 Aug 2023 16:02:46 -0700
From: Doug Anderson <dianders@...omium.org>
To: Stephen Boyd <swboyd@...omium.org>
Cc: Catalin Marinas <catalin.marinas@....com>,
Daniel Thompson <daniel.thompson@...aro.org>,
Marc Zyngier <maz@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Sumit Garg <sumit.garg@...aro.org>,
Will Deacon <will@...nel.org>,
"Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
Ard Biesheuvel <ardb@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
kgdb-bugreport@...ts.sourceforge.net,
Lecopzer Chen <lecopzer.chen@...iatek.com>,
linux-perf-users@...r.kernel.org,
Masayoshi Mizuma <msys.mizuma@...il.com>,
Chen-Yu Tsai <wens@...e.org>,
linux-arm-kernel@...ts.infradead.org, ito-yuichi@...itsu.com,
Peter Zijlstra <peterz@...radead.org>,
D Scott Phillips <scott@...amperecomputing.com>,
Ingo Molnar <mingo@...nel.org>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Valentin Schneider <vschneid@...hat.com>,
linux-kernel@...r.kernel.org
Subject: 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 <swboyd@...omium.org> wrote:
>
> Quoting Douglas Anderson (2023-08-24 08:30:30)
> > 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 <swboyd@...omium.org>
>
> > +#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.
> > 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.
> > +{
> > + /*
> > + * 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
Powered by blists - more mailing lists