| Message ID | 1421166931-14134-6-git-send-email-daniel.thompson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday, January 13, 2015 04:35:31 PM Daniel Thompson wrote: > Using FIQ (if it is available) gives perf a better insight into the > system by allowing code run with interrupts disabled to be profiled. > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> > --- > arch/arm/include/asm/pmu.h | 4 ++++ > arch/arm/kernel/perf_event.c | 2 +- > arch/arm/kernel/perf_event_cpu.c | 35 ++++++++++++++++++++++++++++++++--- > arch/arm/kernel/traps.c | 3 ++- > 4 files changed, 39 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h > index b1596bd59129..2a7ea97a4a14 100644 > --- a/arch/arm/include/asm/pmu.h > +++ b/arch/arm/include/asm/pmu.h > @@ -123,6 +123,8 @@ struct arm_pmu { > > extern const struct dev_pm_ops armpmu_dev_pm_ops; > > +irqreturn_t armpmu_dispatch_irq(int irq, void *dev); > + > int armpmu_register(struct arm_pmu *armpmu, int type); > > u64 armpmu_event_update(struct perf_event *event); > @@ -136,6 +138,8 @@ int armpmu_map_event(struct perf_event *event, > [PERF_COUNT_HW_CACHE_RESULT_MAX], > u32 raw_event_mask); > > +void cpu_pmu_handle_fiq(int irq); > + > struct pmu_probe_info { > unsigned int cpuid; > unsigned int mask; > diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c > index f7c65adaa428..5ae9adf7f18e 100644 > --- a/arch/arm/kernel/perf_event.c > +++ b/arch/arm/kernel/perf_event.c > @@ -296,7 +296,7 @@ validate_group(struct perf_event *event) > return 0; > } > > -static irqreturn_t armpmu_dispatch_irq(int irq, void *dev) > +irqreturn_t armpmu_dispatch_irq(int irq, void *dev) > { > struct arm_pmu *armpmu; > struct platform_device *plat_device; > diff --git a/arch/arm/kernel/perf_event_cpu.c > b/arch/arm/kernel/perf_event_cpu.c index a80309087a7b..5c4e9ce23389 100644 > --- a/arch/arm/kernel/perf_event_cpu.c > +++ b/arch/arm/kernel/perf_event_cpu.c > @@ -36,6 +36,9 @@ > /* Set at runtime when we know what CPU type we are. */ > static struct arm_pmu *cpu_pmu; > > +/* Allows us to find out if an IRQ is for us (mostly used from NMI context) > */ +static DEFINE_PER_CPU(int, cpu_pmu_irqs); > + > /* > * Despite the names, these two functions are CPU-specific and are used > * by the OProfile/perf code. > @@ -127,6 +130,24 @@ static void cpu_pmu_free_irq(struct arm_pmu *cpu_pmu) > } > } > > +/* > + * This handler is called *unconditionally* from the default NMI/FIQ > + * handler. The irq may not be anything to do with us so the main > + * job of this function is to figure out if the irq passed in is ours > + * or not. > + */ This comment is an indicator that all the code in cpu_pmu_handle_fiq is in the wrong place. It (or something like it) belongs at the level of the default FIQ handler, and not in perf_event_cpu.c > +void cpu_pmu_handle_fiq(int irq) > +{ > + int cpu = smp_processor_id(); > + > + if (irq != get_cpu_var(cpu_pmu_irqs)) > + return; > + > + (void)armpmu_dispatch_irq(irq, > + get_cpu_ptr(&cpu_pmu->hw_events->percpu_pmu)); > +} > + > + > static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t > handler) { > int i, err, irq, irqs; > @@ -170,9 +191,16 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, > irq_handler_t handler) continue; > } > > - err = request_irq(irq, handler, > - IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu", > - per_cpu_ptr(&hw_events->percpu_pmu, i)); > + err = request_nmi_irq( > + irq, IRQF_NOBALANCING, "arm-pmu", > + per_cpu_ptr(&hw_events->percpu_pmu, i)); > + if (err) { > + err = request_irq( > + irq, handler, > + IRQF_NOBALANCING | IRQF_NO_THREAD, > + "arm-pmu", > + per_cpu_ptr(&hw_events->percpu_pmu, i)); > + } > if (err) { > pr_err("unable to request IRQ%d for ARM PMU counters\n", > irq); > @@ -180,6 +208,7 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, > irq_handler_t handler) } > > cpumask_set_cpu(i, &cpu_pmu->active_irqs); > + per_cpu(cpu_pmu_irqs, i) = irq; > } > } > > diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > index 74c752b9db68..c581e07517ff 100644 > --- a/arch/arm/kernel/traps.c > +++ b/arch/arm/kernel/traps.c > @@ -38,6 +38,7 @@ > #include <asm/tls.h> > #include <asm/system_misc.h> > #include <asm/opcodes.h> > +#include <asm/pmu.h> > > > static const char *handler[]= { > @@ -485,7 +486,7 @@ asmlinkage void __exception_irq_entry > handle_fiq_as_nmi(struct pt_regs *regs) irq = gic_ack_fiq(); > > if (irq) { > - /* empty - no SPI handlers (yet) */ > + cpu_pmu_handle_fiq(irq); > } else { > #ifdef CONFIG_SMP > ipi_cpu_backtrace(regs); > -- > 1.9.3 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tue, Jan 13, 2015 at 04:35:31PM +0000, Daniel Thompson wrote: > +/* > + * This handler is called *unconditionally* from the default NMI/FIQ > + * handler. The irq may not be anything to do with us so the main > + * job of this function is to figure out if the irq passed in is ours > + * or not. > + */ > +void cpu_pmu_handle_fiq(int irq) > +{ > + int cpu = smp_processor_id(); This can be either debug_smp_processor_id() or raw_smp_processor_id(). raw_smp_processor_id() is fine from FIQ contexts, as seems to be debug_smp_processor_id(), but only because we guarantee that irqs_disabled() in there will be true. > + > + if (irq != get_cpu_var(cpu_pmu_irqs)) > + return; get_cpu_var() needs put_cpu_var() to undo its effects. get_cpu_var() calls preempt_disable(), which calls into lockdep... I think we determined that was fine last time we went digging? put_cpu_var() would call preempt_enable() which I'd hope would be safe in FIQ/NMI contexts? > + > + (void)armpmu_dispatch_irq(irq, > + get_cpu_ptr(&cpu_pmu->hw_events->percpu_pmu)); Again, get_cpu_xxx() needs to be balanced with a put_cpu_xxx().
On 19/01/15 17:48, Russell King - ARM Linux wrote: > On Tue, Jan 13, 2015 at 04:35:31PM +0000, Daniel Thompson wrote: >> +/* >> + * This handler is called *unconditionally* from the default NMI/FIQ >> + * handler. The irq may not be anything to do with us so the main >> + * job of this function is to figure out if the irq passed in is ours >> + * or not. >> + */ >> +void cpu_pmu_handle_fiq(int irq) >> +{ >> + int cpu = smp_processor_id(); > > This can be either debug_smp_processor_id() or raw_smp_processor_id(). > raw_smp_processor_id() is fine from FIQ contexts, as seems to be > debug_smp_processor_id(), but only because we guarantee that > irqs_disabled() in there will be true. Curiously I was looking at exactly this yesterday (because I was intrigued why the NMI-safe bits of kgdb use raw_smp_processor_id() but the x86 arch_trigger_all_cpu_backtrace() implementation uses smp_processor_id()). Given the comments make clear smp_processor_id() is the preferred variant except for false positives I concluded I would continue with smp_processor_id() for any code I write hanging off the default FIQ handler. No objections? >> + >> + if (irq != get_cpu_var(cpu_pmu_irqs)) >> + return; > > get_cpu_var() needs put_cpu_var() to undo its effects. get_cpu_var() > calls preempt_disable(), which calls into lockdep... I think we > determined that was fine last time we went digging? Yes. We reviewed lockdep from the point-of-view of RCU and found that lockdep disabled most of itself when in_nmi() is true. > put_cpu_var() > would call preempt_enable() which I'd hope would be safe in FIQ/NMI > contexts? Yes. preempt_count_add/sub form part of the work done by nmi_enter() and nmi_exit(). However this code gets no benefit from calling get_cpu_var(). I think it would be better to switch it to this_cpu_ptr. >> + >> + (void)armpmu_dispatch_irq(irq, >> + get_cpu_ptr(&cpu_pmu->hw_events->percpu_pmu)); > > Again, get_cpu_xxx() needs to be balanced with a put_cpu_xxx(). > >
On 19/01/15 16:35, Joshua Clayton wrote: > On Tuesday, January 13, 2015 04:35:31 PM Daniel Thompson wrote: >> Using FIQ (if it is available) gives perf a better insight into the >> system by allowing code run with interrupts disabled to be profiled. >> >> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> >> --- >> arch/arm/include/asm/pmu.h | 4 ++++ >> arch/arm/kernel/perf_event.c | 2 +- >> arch/arm/kernel/perf_event_cpu.c | 35 ++++++++++++++++++++++++++++++++--- >> arch/arm/kernel/traps.c | 3 ++- >> 4 files changed, 39 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h >> index b1596bd59129..2a7ea97a4a14 100644 >> --- a/arch/arm/include/asm/pmu.h >> +++ b/arch/arm/include/asm/pmu.h >> @@ -123,6 +123,8 @@ struct arm_pmu { >> >> extern const struct dev_pm_ops armpmu_dev_pm_ops; >> >> +irqreturn_t armpmu_dispatch_irq(int irq, void *dev); >> + >> int armpmu_register(struct arm_pmu *armpmu, int type); >> >> u64 armpmu_event_update(struct perf_event *event); >> @@ -136,6 +138,8 @@ int armpmu_map_event(struct perf_event *event, >> [PERF_COUNT_HW_CACHE_RESULT_MAX], >> u32 raw_event_mask); >> >> +void cpu_pmu_handle_fiq(int irq); >> + >> struct pmu_probe_info { >> unsigned int cpuid; >> unsigned int mask; >> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c >> index f7c65adaa428..5ae9adf7f18e 100644 >> --- a/arch/arm/kernel/perf_event.c >> +++ b/arch/arm/kernel/perf_event.c >> @@ -296,7 +296,7 @@ validate_group(struct perf_event *event) >> return 0; >> } >> >> -static irqreturn_t armpmu_dispatch_irq(int irq, void *dev) >> +irqreturn_t armpmu_dispatch_irq(int irq, void *dev) >> { >> struct arm_pmu *armpmu; >> struct platform_device *plat_device; >> diff --git a/arch/arm/kernel/perf_event_cpu.c >> b/arch/arm/kernel/perf_event_cpu.c index a80309087a7b..5c4e9ce23389 100644 >> --- a/arch/arm/kernel/perf_event_cpu.c >> +++ b/arch/arm/kernel/perf_event_cpu.c >> @@ -36,6 +36,9 @@ >> /* Set at runtime when we know what CPU type we are. */ >> static struct arm_pmu *cpu_pmu; >> >> +/* Allows us to find out if an IRQ is for us (mostly used from NMI context) >> */ +static DEFINE_PER_CPU(int, cpu_pmu_irqs); >> + >> /* >> * Despite the names, these two functions are CPU-specific and are used >> * by the OProfile/perf code. >> @@ -127,6 +130,24 @@ static void cpu_pmu_free_irq(struct arm_pmu *cpu_pmu) >> } >> } >> >> +/* >> + * This handler is called *unconditionally* from the default NMI/FIQ >> + * handler. The irq may not be anything to do with us so the main >> + * job of this function is to figure out if the irq passed in is ours >> + * or not. >> + */ > > This comment is an indicator that all the code in cpu_pmu_handle_fiq is > in the wrong place. > It (or something like it) belongs at the level of the default > FIQ handler, and not in perf_event_cpu.c I'm not sure about that. If we moved this code into the default FIQ handler that means the PMU driver would have to explicitly share its irq value with the default FIQ handler (which doesn't really care about that). I'm inclined to view this code as the effect of avoiding indirection in the default FIQ handler. Regular irq code has nothing like this because &armpmu_dispatch_irq, irq and get_cpu_ptr(&cpu_pmu->hw_events->percpu_pmu) would all be looked up from the irqaction and any unwanted events are naturally filtered by the irq dispatch. After your review I'm very tempted to put together a patch that dispatches NMIs indirectly from the default FIQ handler. However I still don't have much of an answer to Russell's concerns about code review. >> +void cpu_pmu_handle_fiq(int irq) >> +{ >> + int cpu = smp_processor_id(); >> + >> + if (irq != get_cpu_var(cpu_pmu_irqs)) >> + return; >> + >> + (void)armpmu_dispatch_irq(irq, >> + get_cpu_ptr(&cpu_pmu->hw_events->percpu_pmu)); >> +} >> + >> + >> static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t >> handler) { >> int i, err, irq, irqs; >> @@ -170,9 +191,16 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, >> irq_handler_t handler) continue; >> } >> >> - err = request_irq(irq, handler, >> - IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu", >> - per_cpu_ptr(&hw_events->percpu_pmu, i)); >> + err = request_nmi_irq( >> + irq, IRQF_NOBALANCING, "arm-pmu", >> + per_cpu_ptr(&hw_events->percpu_pmu, i)); >> + if (err) { >> + err = request_irq( >> + irq, handler, >> + IRQF_NOBALANCING | IRQF_NO_THREAD, >> + "arm-pmu", >> + per_cpu_ptr(&hw_events->percpu_pmu, i)); >> + } >> if (err) { >> pr_err("unable to request IRQ%d for ARM PMU counters\n", >> irq); >> @@ -180,6 +208,7 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, >> irq_handler_t handler) } >> >> cpumask_set_cpu(i, &cpu_pmu->active_irqs); >> + per_cpu(cpu_pmu_irqs, i) = irq; >> } >> } >> >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c >> index 74c752b9db68..c581e07517ff 100644 >> --- a/arch/arm/kernel/traps.c >> +++ b/arch/arm/kernel/traps.c >> @@ -38,6 +38,7 @@ >> #include <asm/tls.h> >> #include <asm/system_misc.h> >> #include <asm/opcodes.h> >> +#include <asm/pmu.h> >> >> >> static const char *handler[]= { >> @@ -485,7 +486,7 @@ asmlinkage void __exception_irq_entry >> handle_fiq_as_nmi(struct pt_regs *regs) irq = gic_ack_fiq(); >> >> if (irq) { >> - /* empty - no SPI handlers (yet) */ >> + cpu_pmu_handle_fiq(irq); >> } else { >> #ifdef CONFIG_SMP >> ipi_cpu_backtrace(regs); >> -- >> 1.9.3 >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Tuesday, January 20, 2015 10:18:10 AM Daniel Thompson wrote: > On 19/01/15 16:35, Joshua Clayton wrote: > > On Tuesday, January 13, 2015 04:35:31 PM Daniel Thompson wrote: > >> Using FIQ (if it is available) gives perf a better insight into the > >> system by allowing code run with interrupts disabled to be profiled. > >> > >> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> > >> --- > >> > >> arch/arm/include/asm/pmu.h | 4 ++++ > >> arch/arm/kernel/perf_event.c | 2 +- > >> arch/arm/kernel/perf_event_cpu.c | 35 > >> ++++++++++++++++++++++++++++++++--- > >> arch/arm/kernel/traps.c | 3 ++- > >> 4 files changed, 39 insertions(+), 5 deletions(-) > >> > >> diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h > >> index b1596bd59129..2a7ea97a4a14 100644 > >> --- a/arch/arm/include/asm/pmu.h > >> +++ b/arch/arm/include/asm/pmu.h > >> @@ -123,6 +123,8 @@ struct arm_pmu { > >> > >> extern const struct dev_pm_ops armpmu_dev_pm_ops; > >> > >> +irqreturn_t armpmu_dispatch_irq(int irq, void *dev); > >> + > >> > >> int armpmu_register(struct arm_pmu *armpmu, int type); > >> > >> u64 armpmu_event_update(struct perf_event *event); > >> > >> @@ -136,6 +138,8 @@ int armpmu_map_event(struct perf_event *event, > >> > >> [PERF_COUNT_HW_CACHE_RESULT_MAX], > >> > >> u32 raw_event_mask); > >> > >> +void cpu_pmu_handle_fiq(int irq); > >> + > >> > >> struct pmu_probe_info { > >> > >> unsigned int cpuid; > >> unsigned int mask; > >> > >> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c > >> index f7c65adaa428..5ae9adf7f18e 100644 > >> --- a/arch/arm/kernel/perf_event.c > >> +++ b/arch/arm/kernel/perf_event.c > >> @@ -296,7 +296,7 @@ validate_group(struct perf_event *event) > >> > >> return 0; > >> > >> } > >> > >> -static irqreturn_t armpmu_dispatch_irq(int irq, void *dev) > >> +irqreturn_t armpmu_dispatch_irq(int irq, void *dev) > >> > >> { > >> > >> struct arm_pmu *armpmu; > >> struct platform_device *plat_device; > >> > >> diff --git a/arch/arm/kernel/perf_event_cpu.c > >> b/arch/arm/kernel/perf_event_cpu.c index a80309087a7b..5c4e9ce23389 > >> 100644 > >> --- a/arch/arm/kernel/perf_event_cpu.c > >> +++ b/arch/arm/kernel/perf_event_cpu.c > >> @@ -36,6 +36,9 @@ > >> > >> /* Set at runtime when we know what CPU type we are. */ > >> static struct arm_pmu *cpu_pmu; > >> > >> +/* Allows us to find out if an IRQ is for us (mostly used from NMI > >> context) */ +static DEFINE_PER_CPU(int, cpu_pmu_irqs); > >> + > >> > >> /* > >> > >> * Despite the names, these two functions are CPU-specific and are used > >> * by the OProfile/perf code. > >> > >> @@ -127,6 +130,24 @@ static void cpu_pmu_free_irq(struct arm_pmu > >> *cpu_pmu) > >> > >> } > >> > >> } > >> > >> +/* > >> + * This handler is called *unconditionally* from the default NMI/FIQ > >> + * handler. The irq may not be anything to do with us so the main > >> + * job of this function is to figure out if the irq passed in is ours > >> + * or not. > >> + */ > > > > This comment is an indicator that all the code in cpu_pmu_handle_fiq is > > in the wrong place. > > It (or something like it) belongs at the level of the default > > FIQ handler, and not in perf_event_cpu.c > > I'm not sure about that. > > If we moved this code into the default FIQ handler that means the PMU > driver would have to explicitly share its irq value with the default FIQ > handler (which doesn't really care about that). > > I'm inclined to view this code as the effect of avoiding indirection in > the default FIQ handler. > > Regular irq code has nothing like this because &armpmu_dispatch_irq, irq > and get_cpu_ptr(&cpu_pmu->hw_events->percpu_pmu) would all be looked up > from the irqaction and any unwanted events are naturally filtered by the > irq dispatch. > > After your review I'm very tempted to put together a patch that > dispatches NMIs indirectly from the default FIQ handler. However I still > don't have much of an answer to Russell's concerns about code review. > Ironically I'm exactly the person RMK wants to protect against. Perhaps request_nmi_irq (or code called from there) might be the best place to put a "bad code filter", rather than in the handler/dispatcher as the FIQ is running. > >> +void cpu_pmu_handle_fiq(int irq) > >> +{ > >> + int cpu = smp_processor_id(); > >> + > >> + if (irq != get_cpu_var(cpu_pmu_irqs)) > >> + return; > >> + > >> + (void)armpmu_dispatch_irq(irq, > >> + get_cpu_ptr(&cpu_pmu->hw_events->percpu_pmu)); > >> +} > >> + > >> + > >> > >> static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t > >> > >> handler) { > >> > >> int i, err, irq, irqs; > >> > >> @@ -170,9 +191,16 @@ static int cpu_pmu_request_irq(struct arm_pmu > >> *cpu_pmu, irq_handler_t handler) continue; > >> > >> } > >> > >> - err = request_irq(irq, handler, > >> - IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu", > >> - per_cpu_ptr(&hw_events->percpu_pmu, i)); > >> + err = request_nmi_irq( > >> + irq, IRQF_NOBALANCING, "arm-pmu", > >> + per_cpu_ptr(&hw_events->percpu_pmu, i)); > >> + if (err) { > >> + err = request_irq( > >> + irq, handler, > >> + IRQF_NOBALANCING | IRQF_NO_THREAD, > >> + "arm-pmu", > >> + per_cpu_ptr(&hw_events->percpu_pmu, i)); > >> + } > >> > >> if (err) { > >> > >> pr_err("unable to request IRQ%d for ARM PMU counters\n", > >> > >> irq); > >> > >> @@ -180,6 +208,7 @@ static int cpu_pmu_request_irq(struct arm_pmu > >> *cpu_pmu, > >> irq_handler_t handler) } > >> > >> cpumask_set_cpu(i, &cpu_pmu->active_irqs); > >> > >> + per_cpu(cpu_pmu_irqs, i) = irq; > >> > >> } > >> > >> } > >> > >> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c > >> index 74c752b9db68..c581e07517ff 100644 > >> --- a/arch/arm/kernel/traps.c > >> +++ b/arch/arm/kernel/traps.c > >> @@ -38,6 +38,7 @@ > >> > >> #include <asm/tls.h> > >> #include <asm/system_misc.h> > >> #include <asm/opcodes.h> > >> > >> +#include <asm/pmu.h> > >> > >> static const char *handler[]= { > >> > >> @@ -485,7 +486,7 @@ asmlinkage void __exception_irq_entry > >> handle_fiq_as_nmi(struct pt_regs *regs) irq = gic_ack_fiq(); > >> > >> if (irq) { > >> > >> - /* empty - no SPI handlers (yet) */ > >> + cpu_pmu_handle_fiq(irq); > >> > >> } else { > >> > >> #ifdef CONFIG_SMP > >> > >> ipi_cpu_backtrace(regs); > >> > >> -- > >> 1.9.3 > >> > >> > >> _______________________________________________ > >> linux-arm-kernel mailing list > >> linux-arm-kernel@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h index b1596bd59129..2a7ea97a4a14 100644 --- a/arch/arm/include/asm/pmu.h +++ b/arch/arm/include/asm/pmu.h @@ -123,6 +123,8 @@ struct arm_pmu { extern const struct dev_pm_ops armpmu_dev_pm_ops; +irqreturn_t armpmu_dispatch_irq(int irq, void *dev); + int armpmu_register(struct arm_pmu *armpmu, int type); u64 armpmu_event_update(struct perf_event *event); @@ -136,6 +138,8 @@ int armpmu_map_event(struct perf_event *event, [PERF_COUNT_HW_CACHE_RESULT_MAX], u32 raw_event_mask); +void cpu_pmu_handle_fiq(int irq); + struct pmu_probe_info { unsigned int cpuid; unsigned int mask; diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c index f7c65adaa428..5ae9adf7f18e 100644 --- a/arch/arm/kernel/perf_event.c +++ b/arch/arm/kernel/perf_event.c @@ -296,7 +296,7 @@ validate_group(struct perf_event *event) return 0; } -static irqreturn_t armpmu_dispatch_irq(int irq, void *dev) +irqreturn_t armpmu_dispatch_irq(int irq, void *dev) { struct arm_pmu *armpmu; struct platform_device *plat_device; diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c index a80309087a7b..5c4e9ce23389 100644 --- a/arch/arm/kernel/perf_event_cpu.c +++ b/arch/arm/kernel/perf_event_cpu.c @@ -36,6 +36,9 @@ /* Set at runtime when we know what CPU type we are. */ static struct arm_pmu *cpu_pmu; +/* Allows us to find out if an IRQ is for us (mostly used from NMI context) */ +static DEFINE_PER_CPU(int, cpu_pmu_irqs); + /* * Despite the names, these two functions are CPU-specific and are used * by the OProfile/perf code. @@ -127,6 +130,24 @@ static void cpu_pmu_free_irq(struct arm_pmu *cpu_pmu) } } +/* + * This handler is called *unconditionally* from the default NMI/FIQ + * handler. The irq may not be anything to do with us so the main + * job of this function is to figure out if the irq passed in is ours + * or not. + */ +void cpu_pmu_handle_fiq(int irq) +{ + int cpu = smp_processor_id(); + + if (irq != get_cpu_var(cpu_pmu_irqs)) + return; + + (void)armpmu_dispatch_irq(irq, + get_cpu_ptr(&cpu_pmu->hw_events->percpu_pmu)); +} + + static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler) { int i, err, irq, irqs; @@ -170,9 +191,16 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler) continue; } - err = request_irq(irq, handler, - IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu", - per_cpu_ptr(&hw_events->percpu_pmu, i)); + err = request_nmi_irq( + irq, IRQF_NOBALANCING, "arm-pmu", + per_cpu_ptr(&hw_events->percpu_pmu, i)); + if (err) { + err = request_irq( + irq, handler, + IRQF_NOBALANCING | IRQF_NO_THREAD, + "arm-pmu", + per_cpu_ptr(&hw_events->percpu_pmu, i)); + } if (err) { pr_err("unable to request IRQ%d for ARM PMU counters\n", irq); @@ -180,6 +208,7 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler) } cpumask_set_cpu(i, &cpu_pmu->active_irqs); + per_cpu(cpu_pmu_irqs, i) = irq; } } diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c index 74c752b9db68..c581e07517ff 100644 --- a/arch/arm/kernel/traps.c +++ b/arch/arm/kernel/traps.c @@ -38,6 +38,7 @@ #include <asm/tls.h> #include <asm/system_misc.h> #include <asm/opcodes.h> +#include <asm/pmu.h> static const char *handler[]= { @@ -485,7 +486,7 @@ asmlinkage void __exception_irq_entry handle_fiq_as_nmi(struct pt_regs *regs) irq = gic_ack_fiq(); if (irq) { - /* empty - no SPI handlers (yet) */ + cpu_pmu_handle_fiq(irq); } else { #ifdef CONFIG_SMP ipi_cpu_backtrace(regs);
Using FIQ (if it is available) gives perf a better insight into the system by allowing code run with interrupts disabled to be profiled. Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> --- arch/arm/include/asm/pmu.h | 4 ++++ arch/arm/kernel/perf_event.c | 2 +- arch/arm/kernel/perf_event_cpu.c | 35 ++++++++++++++++++++++++++++++++--- arch/arm/kernel/traps.c | 3 ++- 4 files changed, 39 insertions(+), 5 deletions(-)