| Message ID | 1563351432-55652-7-git-send-email-julien.thierry@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm_pmu: Use NMI for perf interrupt | expand |
On Wed, Jul 17, 2019 at 09:17:09AM +0100, Julien Thierry wrote: > Function irq_work_run is not NMI safe and should not be called from NMI > context. > > When PMU interrupt is an NMI do not call irq_work_run. Instead rely on the > IRQ work IPI to run the irq_work queue once NMI/IRQ contexts have been > exited. > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > --- > arch/arm64/kernel/perf_event.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c > index 0e2cf5d..9c959ad 100644 > --- a/arch/arm64/kernel/perf_event.c > +++ b/arch/arm64/kernel/perf_event.c > @@ -776,20 +776,16 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu) > if (!armpmu_event_set_period(event)) > continue; > > + /* > + * Perf event overflow will queue the processing of the event as > + * an irq_work which will be taken care of in the handling of > + * IPI_IRQ_WORK. > + */ > if (perf_event_overflow(event, &data, regs)) > cpu_pmu->disable(event); > } > armv8pmu_start(cpu_pmu); > > - /* > - * Handle the pending perf events. > - * > - * Note: this call *must* be run with interrupts disabled. For > - * platforms that can have the PMU interrupts raised as an NMI, this > - * will not work. > - */ > - irq_work_run(); What about the case where NMIs are not being used (e.g. GICv2)? Will
On 01/08/2019 14:06, Will Deacon wrote: > On Wed, Jul 17, 2019 at 09:17:09AM +0100, Julien Thierry wrote: >> Function irq_work_run is not NMI safe and should not be called from NMI >> context. >> >> When PMU interrupt is an NMI do not call irq_work_run. Instead rely on the >> IRQ work IPI to run the irq_work queue once NMI/IRQ contexts have been >> exited. >> >> Signed-off-by: Julien Thierry <julien.thierry@arm.com> >> Cc: Will Deacon <will.deacon@arm.com> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Peter Zijlstra <peterz@infradead.org> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> >> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> >> Cc: Jiri Olsa <jolsa@redhat.com> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> --- >> arch/arm64/kernel/perf_event.c | 14 +++++--------- >> 1 file changed, 5 insertions(+), 9 deletions(-) >> >> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c >> index 0e2cf5d..9c959ad 100644 >> --- a/arch/arm64/kernel/perf_event.c >> +++ b/arch/arm64/kernel/perf_event.c >> @@ -776,20 +776,16 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu) >> if (!armpmu_event_set_period(event)) >> continue; >> >> + /* >> + * Perf event overflow will queue the processing of the event as >> + * an irq_work which will be taken care of in the handling of >> + * IPI_IRQ_WORK. >> + */ >> if (perf_event_overflow(event, &data, regs)) >> cpu_pmu->disable(event); >> } >> armv8pmu_start(cpu_pmu); >> >> - /* >> - * Handle the pending perf events. >> - * >> - * Note: this call *must* be run with interrupts disabled. For >> - * platforms that can have the PMU interrupts raised as an NMI, this >> - * will not work. >> - */ >> - irq_work_run(); > > What about the case where NMIs are not being used (e.g. GICv2)? > As the comment above mentions. The overflow handler will trigger the IPI_IRQ_WORK which will call the irq_work_run() both for NMI and normal IRQ. Unless we really need to process the irq_work here, it makes things simpler to get rid of the call. It was suggested during the previous version: https://www.spinics.net/lists/arm-kernel/msg740027.html Cheers,
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c index 0e2cf5d..9c959ad 100644 --- a/arch/arm64/kernel/perf_event.c +++ b/arch/arm64/kernel/perf_event.c @@ -776,20 +776,16 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu) if (!armpmu_event_set_period(event)) continue; + /* + * Perf event overflow will queue the processing of the event as + * an irq_work which will be taken care of in the handling of + * IPI_IRQ_WORK. + */ if (perf_event_overflow(event, &data, regs)) cpu_pmu->disable(event); } armv8pmu_start(cpu_pmu); - /* - * Handle the pending perf events. - * - * Note: this call *must* be run with interrupts disabled. For - * platforms that can have the PMU interrupts raised as an NMI, this - * will not work. - */ - irq_work_run(); - return IRQ_HANDLED; }
Function irq_work_run is not NMI safe and should not be called from NMI context. When PMU interrupt is an NMI do not call irq_work_run. Instead rely on the IRQ work IPI to run the irq_work queue once NMI/IRQ contexts have been exited. Signed-off-by: Julien Thierry <julien.thierry@arm.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Catalin Marinas <catalin.marinas@arm.com> --- arch/arm64/kernel/perf_event.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)