Distinguish percpu irq and percpu_devid irq
cover Distinguish percpu irq and percpu_devid irq
[Resending with cover letter + better commit message for patch 1]
Hi,
Looking at the arm_pmu driver, it seems we are using irq_is_percpu when we want to know if an irq must be configured individually on each cpu. But this is not what irq_is_percpu does. The arm_pmu marking all its irqs as PERCPU, driver attempts to enable the irq on each CPU even when it is not needed.
This has not caused issue so far because enable_percpu_irq returns immediatly if the provided irq is not PERCPU_DEVID.
* Patch 1 adds the function to identify PERCPU_DEVID irqs.
* Patch 2 replaces the irq_is_percpu calls with the new function
Cheers,
Julien
Julien Thierry (2):
irqdesc: Add function to identify percpu_devid irqs arm/arm64: pmu: Distinguish percpu irq and percpu_devid irq
drivers/perf/arm_pmu.c | 10 +++++----- drivers/perf/arm_pmu_platform.c | 4 ++-- include/linux/irqdesc.h | 8 ++++++++ 3 files changed, 15 insertions(+), 7 deletions(-)
--
1.9.1
PATCH RESEND 1/2 irqdesc: Add function to identify percpu_devid irqs
irq_is_percpu indicates whether an irq should only target a single cpu. PERCPU_DEVID flag indicates that an irq can be configured differently on each cpu it can target.
Provide a function to check whether an irq is PERCPU_DEVID.
--- include/linux/irqdesc.h | 8 ++++++++ 1 file changed, 8 insertions(+)
include/linux/irqdesc.h +8 -0
@@ -244,6 +244,14 @@ static inline int irq_is_percpu(unsigned int irq)return desc->status_use_accessors &IRQ_PER_CPU ; } +static inline int irq_is_percpu_devid (unsigned int irq) +{ +struct irq_desc *desc; + + desc =irq_to_desc (irq); +return desc->status_use_accessors &IRQ_PER_CPU_DEVID ; +} +static inline void irq_set_lockdep_class (unsigned int irq,struct lock_class_key *class) {
--
1.9.1
PATCH RESEND 1/2 irqdesc: Add function to identify percpu_devid irqs
Quoted 33 lines · Julien Thierry
irq_is_percpuindicates whether an irq should only target a single cpu.PERCPU_DEVIDflag indicates that an irq can be configured differently on each cpu it can target.Provide a function to check whether an irq is
PERCPU_DEVID.Signed-off-by Julien Thierry <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Marc Zyngier <[email protected]>--- include/linux/irqdesc.h | 8 ++++++++ 1 file changed, 8 insertions(+)include/linux/irqdesc.h +8
-1@@ -244,6 +244,14 @@ static inline int irq_is_percpu(unsigned int irq)return desc->status_use_accessors &IRQ_PER_CPU ; } +static inline int irq_is_percpu_devid (unsigned int irq) +{ +struct irq_desc *desc; + + desc =irq_to_desc (irq); +return desc->status_use_accessors &IRQ_PER_CPU_DEVID ; +} +static inline void irq_set_lockdep_class (unsigned int irq,struct lock_class_key *class) { --
Quoted 2 lines
1.9.1
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
Mark Rutland
maintainerPATCH RESEND 1/2 irqdesc: Add function to identify percpu_devid irqs
In reply to Julien ThierryHi Thomas,
Would you be happy for this patch to be taken via the arm64 tree?
The next patch [1] makes use of this in the arm_pmu driver, and we're likely to have some other changes there shortly.
Thanks,
Mark
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-October/536532.html
Quoted 34 lines · Julien Thierry
irq_is_percpuindicates whether an irq should only target a single cpu.PERCPU_DEVIDflag indicates that an irq can be configured differently on each cpu it can target.Provide a function to check whether an irq is
PERCPU_DEVID.Signed-off-by Julien Thierry <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Marc Zyngier <[email protected]>--- include/linux/irqdesc.h | 8 ++++++++ 1 file changed, 8 insertions(+)include/linux/irqdesc.h +8
-1@@ -244,6 +244,14 @@ static inline int irq_is_percpu(unsigned int irq)return desc->status_use_accessors &IRQ_PER_CPU ; } +static inline int irq_is_percpu_devid (unsigned int irq) +{ +struct irq_desc *desc; + + desc =irq_to_desc (irq); +return desc->status_use_accessors &IRQ_PER_CPU_DEVID ; +} +static inline void irq_set_lockdep_class (unsigned int irq,struct lock_class_key *class) { --1.9.1
PATCH RESEND 1/2 irqdesc: Add function to identify percpu_devid irqs
Hi Thomas,
I was hoping to merge this one via arm64 since it now has Marc's Reviewed-by, but I'd really like your Ack on it too, if you don't mind.
Could you take a look, please?
Thanks,
Will
Quoted 45 lines · Mark Rutland
Would you be happy for this patch to be taken via the arm64 tree?
The next patch [1] makes use of this in the
arm_pmudriver, and we're likely to have some other changes there shortly.Thanks,
Mark[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-October/536532.html
Quoted 34 lines · Julien Thierry
irq_is_percpuindicates whether an irq should only target a single cpu.PERCPU_DEVIDflag indicates that an irq can be configured differently on each cpu it can target.Provide a function to check whether an irq is
PERCPU_DEVID.Signed-off-by Julien Thierry <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Marc Zyngier <[email protected]>--- include/linux/irqdesc.h | 8 ++++++++ 1 file changed, 8 insertions(+)include/linux/irqdesc.h +8
-1@@ -244,6 +244,14 @@ static inline int irq_is_percpu(unsigned int irq)return desc->status_use_accessors &IRQ_PER_CPU ; } +static inline int irq_is_percpu_devid (unsigned int irq) +{ +struct irq_desc *desc; + + desc =irq_to_desc (irq); +return desc->status_use_accessors &IRQ_PER_CPU_DEVID ; +} +static inline void irq_set_lockdep_class (unsigned int irq,struct lock_class_key *class) { --1.9.1
PATCH RESEND 1/2 irqdesc: Add function to identify percpu_devid irqs
Quoted 6 lines · Will Deacon
Hi Thomas,
I was hoping to merge this one via arm64 since it now has Marc's Reviewed-by, but I'd really like your Ack on it too, if you don't mind.
Could you take a look, please?
PATCH RESEND 1/2 irqdesc: Add function to identify percpu_devid irqs
Quoted 10 lines · Thomas Gleixner
Quoted 6 lines · Will Deacon
Hi Thomas,
I was hoping to merge this one via arm64 since it now has Marc's Reviewed-by, but I'd really like your Ack on it too, if you don't mind.
Could you take a look, please?
Reviewed-by Thomas Gleixner <[email protected]>
Brill, cheers.
Will
PATCH RESEND 2/2 arm/arm64: pmu: Distinguish percpu irq and percpu_devid irq
In reply to Julien Thierryarm_pmu interrupts are maked as PERCPU even when these are not local physical interrupts to a single CPU. When using non-local interrupts, interrupts marked as PERCPU will not get freed not disabled properly by the PMU driver.
Check if interrupts are local to a single CPU with PERCPU_DEVID since this is what the PMU driver really needs to know.
Will Deacon <[email protected]>
Mark Rutland <[email protected]>
--- drivers/perf/arm_pmu.c | 10 +++++----- drivers/perf/arm_pmu_platform.c | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-)
drivers/perf/arm_pmu.c +5 -5
@@ -539,7 +539,7 @@ void armpmu_free_irq(struct arm_pmu *armpmu, int cpu)if (!cpumask_test_and_clear_cpu (cpu, &armpmu->active_irqs))return ; -if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {free_percpu_irq (irq, &hw_events->percpu_pmu);cpumask_clear (&armpmu->active_irqs);return ;
@@ -565,10 +565,10 @@ int armpmu_request_irq(struct arm_pmu *armpmu, int cpu)if (!irq)return 0 ; -if (irq_is_percpu (irq) &&cpumask_empty (&armpmu->active_irqs)) { +if (irq_is_percpu_devid (irq) &&cpumask_empty (&armpmu->active_irqs)) { err =request_percpu_irq (irq, handler,"arm-pmu" , &hw_events->percpu_pmu); - }else if (irq_is_percpu (irq)) { + }else if (irq_is_percpu_devid (irq)) {int other_cpu =cpumask_first (&armpmu->active_irqs);int other_irq =per_cpu (hw_events->irq, other_cpu);
@@ -649,7 +649,7 @@ static int arm_perf_starting_cpu(unsigned int cpu, struct hlist_node *node) irq =armpmu_get_cpu_irq (pmu, cpu);if (irq) { -if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {enable_percpu_irq (irq,IRQ_TYPE_NONE );return 0 ; }
@@ -667,7 +667,7 @@ static int arm_perf_teardown_cpu(unsigned int cpu, struct hlist_node *node)return 0 ; irq =armpmu_get_cpu_irq (pmu, cpu); -if (irq &&irq_is_percpu (irq)) +if (irq &&irq_is_percpu_devid (irq))disable_percpu_irq (irq);return 0 ;
drivers/perf/arm_pmu_platform.c +2 -2
@@ -126,7 +126,7 @@ static int pmu_parse_irqs(struct arm_pmu *pmu)if (num_irqs ==1 ) {int irq =platform_get_irq (pdev,0 ); -if (irq &&irq_is_percpu (irq)) +if (irq &&irq_is_percpu_devid (irq))return pmu_parse_percpu_irq (pmu, irq); }
@@ -149,7 +149,7 @@ static int pmu_parse_irqs(struct arm_pmu *pmu)if (WARN_ON (irq <=0 ))continue ; -if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {pr_warn ("multiple PPIs or mismatched SPI/PPI detected\n" );return -EINVAL ; }
--
1.9.1
Mark Rutland
maintainerPATCH RESEND 2/2 arm/arm64: pmu: Distinguish percpu irq and percpu_devid irq
Quoted 11 lines · Julien Thierry
arm_pmuinterrupts are maked as PERCPU even when these are not local physical interrupts to a single CPU. When using non-local interrupts, interrupts marked as PERCPU will not get freed not disabled properly by the PMU driver.Check if interrupts are local to a single CPU with
PERCPU_DEVIDsince this is what the PMU driver really needs to know.Signed-off-by Julien Thierry <[email protected]>Will Deacon <[email protected]>
Mark Rutland <[email protected]>
I guess these should have been with prefixed 'Cc: '.
The patch itself looks good to me:
I suspect we'll need to take both patches via the usual arm pmu tree, so it'd be good if we could get an ack on patch 1.
Thanks,
Mark.
Quoted 73 lines
--- drivers/perf/arm_pmu.c | 10 +++++----- drivers/perf/arm_pmu_platform.c | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-)drivers/perf/arm_pmu.c +5
-5@@ -539,7 +539,7 @@ void armpmu_free_irq(struct arm_pmu *armpmu, int cpu)if (!cpumask_test_and_clear_cpu (cpu, &armpmu->active_irqs))return ; -if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {free_percpu_irq (irq, &hw_events->percpu_pmu);cpumask_clear (&armpmu->active_irqs);return ;@@ -565,10 +565,10 @@ int armpmu_request_irq(struct arm_pmu *armpmu, int cpu)if (!irq)return 0 ; -if (irq_is_percpu (irq) &&cpumask_empty (&armpmu->active_irqs)) { +if (irq_is_percpu_devid (irq) &&cpumask_empty (&armpmu->active_irqs)) { err =request_percpu_irq (irq, handler,"arm-pmu" , &hw_events->percpu_pmu); - }else if (irq_is_percpu (irq)) { + }else if (irq_is_percpu_devid (irq)) {int other_cpu =cpumask_first (&armpmu->active_irqs);int other_irq =per_cpu (hw_events->irq, other_cpu);@@ -649,7 +649,7 @@ static int arm_perf_starting_cpu(unsigned int cpu, struct hlist_node *node) irq =armpmu_get_cpu_irq (pmu, cpu);if (irq) { -if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {enable_percpu_irq (irq,IRQ_TYPE_NONE );return 0 ; }@@ -667,7 +667,7 @@ static int arm_perf_teardown_cpu(unsigned int cpu, struct hlist_node *node)return 0 ; irq =armpmu_get_cpu_irq (pmu, cpu); -if (irq &&irq_is_percpu (irq)) +if (irq &&irq_is_percpu_devid (irq))disable_percpu_irq (irq);return 0 ;drivers/perf/arm_pmu_platform.c +2
-3@@ -126,7 +126,7 @@ static int pmu_parse_irqs(struct arm_pmu *pmu)if (num_irqs ==1 ) {int irq =platform_get_irq (pdev,0 ); -if (irq &&irq_is_percpu (irq)) +if (irq &&irq_is_percpu_devid (irq))return pmu_parse_percpu_irq (pmu, irq); }@@ -149,7 +149,7 @@ static int pmu_parse_irqs(struct arm_pmu *pmu)if (WARN_ON (irq <=0 ))continue ; -if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {pr_warn ("multiple PPIs or mismatched SPI/PPI detected\n" );return -EINVAL ; } --1.9.1
PATCH RESEND 2/2 arm/arm64: pmu: Distinguish percpu irq and percpu_devid irq
Quoted 6 lines · Mark Rutland
Quoted 5 lines · Julien Thierry
arm_pmuinterrupts are maked as PERCPU even when these are not local physical interrupts to a single CPU. When using non-local interrupts, interrupts marked as PERCPU will not get freed not disabled properly by the PMU driver.
Quoted 8 lines
Quoted 6 lines
Check if interrupts are local to a single CPU with
PERCPU_DEVIDsince this is what the PMU driver really needs to know.Signed-off-by Julien Thierry <[email protected]>Will Deacon <[email protected]>
Mark Rutland <[email protected]>I guess these should have been with prefixed 'Cc: '.
Oops, yes they should've.
Quoted 8 lines
The patch itself looks good to me:
Acked-by Mark Rutland <[email protected]>I suspect we'll need to take both patches via the usual arm pmu tree, so it'd be good if we could get an ack on patch 1.
Thanks!
Quoted 69 lines
Thanks,
Mark.Quoted 66 lines
--- drivers/perf/arm_pmu.c | 10 +++++----- drivers/perf/arm_pmu_platform.c | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-)drivers/perf/arm_pmu.c +5
-5@@ -539,7 +539,7 @@ void armpmu_free_irq(struct arm_pmu *armpmu, int cpu)if (!cpumask_test_and_clear_cpu (cpu, &armpmu->active_irqs))return ; -if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {free_percpu_irq (irq, &hw_events->percpu_pmu);cpumask_clear (&armpmu->active_irqs);return ;@@ -565,10 +565,10 @@ int armpmu_request_irq(struct arm_pmu *armpmu, int cpu)if (!irq)return 0 ; -if (irq_is_percpu (irq) &&cpumask_empty (&armpmu->active_irqs)) { +if (irq_is_percpu_devid (irq) &&cpumask_empty (&armpmu->active_irqs)) { err =request_percpu_irq (irq, handler,"arm-pmu" , &hw_events->percpu_pmu); - }else if (irq_is_percpu (irq)) { + }else if (irq_is_percpu_devid (irq)) {int other_cpu =cpumask_first (&armpmu->active_irqs);int other_irq =per_cpu (hw_events->irq, other_cpu);@@ -649,7 +649,7 @@ static int arm_perf_starting_cpu(unsigned int cpu, struct hlist_node *node) irq =armpmu_get_cpu_irq (pmu, cpu);if (irq) { -if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {enable_percpu_irq (irq,IRQ_TYPE_NONE );return 0 ; }@@ -667,7 +667,7 @@ static int arm_perf_teardown_cpu(unsigned int cpu, struct hlist_node *node)return 0 ; irq =armpmu_get_cpu_irq (pmu, cpu); -if (irq &&irq_is_percpu (irq)) +if (irq &&irq_is_percpu_devid (irq))disable_percpu_irq (irq);return 0 ;drivers/perf/arm_pmu_platform.c +1
-1@@ -126,7 +126,7 @@ static int pmu_parse_irqs(struct arm_pmu *pmu)if (num_irqs ==1 ) {int irq =platform_get_irq (pdev,0 ); -if (irq &&irq_is_percpu (irq)) +if (irq &&irq_is_percpu_devid (irq))return pmu_parse_percpu_irq (pmu, irq); }@@ -149,7 +149,7 @@ static int pmu_parse_irqs(struct arm_pmu *pmu)if (WARN_ON (irq <=0 ))continue ;
Quoted 7 lines
Quoted 7 lines
drivers/perf/arm_pmu_platform.c +1
-2-if (irq_is_percpu (irq)) { +if (irq_is_percpu_devid (irq)) {pr_warn ("multiple PPIs or mismatched SPI/PPI detected\n" );return -EINVAL ; } --1.9.1
--
Julien Thierry
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.