| Message ID | 1562596377-33196-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 Mon, Jul 08, 2019 at 03:32:54PM +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 | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c > index 878c142..a622139 100644 > --- a/arch/arm64/kernel/perf_event.c > +++ b/arch/arm64/kernel/perf_event.c > @@ -794,7 +794,8 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu) > * platforms that can have the PMU interrupts raised as an NMI, this > * will not work. > */ > - irq_work_run(); > + if (!in_nmi()) > + irq_work_run(); Could we always defer the IRQ work, or would that be problematic for non-NMI? We should probably update the comment to explain why this is safe in either case. Thanks, Mark. > > return IRQ_HANDLED; > } > -- > 1.9.1 >
On 08/07/2019 16:29, Mark Rutland wrote: > On Mon, Jul 08, 2019 at 03:32:54PM +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 | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c >> index 878c142..a622139 100644 >> --- a/arch/arm64/kernel/perf_event.c >> +++ b/arch/arm64/kernel/perf_event.c >> @@ -794,7 +794,8 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu) >> * platforms that can have the PMU interrupts raised as an NMI, this >> * will not work. >> */ >> - irq_work_run(); >> + if (!in_nmi()) >> + irq_work_run(); > > Could we always defer the IRQ work, or would that be problematic for > non-NMI? > In terms of functionality, we can always defer it. It will just delay the event processing (and possibly re-enabling) to the moment where the IRQ work IPI is taken. If I remember correctly, queuing irq_works will send the IPI request anyway, the only difference is how much work will be done in the IPI handler... Personally I think it makes sense to do it always rely on the IPI. > We should probably update the comment to explain why this is safe in > either case. > I'll add a comment where the irq_work is queued. Thanks,
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c index 878c142..a622139 100644 --- a/arch/arm64/kernel/perf_event.c +++ b/arch/arm64/kernel/perf_event.c @@ -794,7 +794,8 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu) * platforms that can have the PMU interrupts raised as an NMI, this * will not work. */ - irq_work_run(); + if (!in_nmi()) + 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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)