* [PATCH V6 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-12-02 9:34 Vinayak Kale
2013-12-02 9:34 ` [PATCH V6 1/2] genirq: Add an accessor for IRQ_PER_CPU flag Vinayak Kale
2013-12-02 9:34 ` [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt Vinayak Kale
0 siblings, 2 replies; 9+ messages in thread
From: Vinayak Kale @ 2013-12-02 9:34 UTC (permalink / raw)
To: linux-arm-kernel
This patch series adds support to handle interrupt registration/deregistration
in arm64 pmu driver when pmu interrupt type is percpu.
Changelog:
V6:
* In arm64 pmu driver: Use macro 'IRQ_TYPE_NONE' while passing irq type value
to enable_percpu_irq(). Change 'irqs' etc variables as unsigned, modify the
check for 'irqs' inside armpmu_[release/reserve]_hardware().
V5:
* In irqdesc.h: Added Chris Smith's sign-off.
In arm64 pmu driver: Handle the invalid irq-0 case for platform_get_irq().
V4:
* In arm64 pmu driver: Avoid using irq_to_desc() to check validity of irq.
V3:
* Remove validity check for 'desc' from accessor function in irqdesc.h .
Instead, check the irq 'desc' validity in arm64 pmu driver.
V2:
* To determine whether an IRQ is percpu or not, added an accessor function in
irqdesc.h . This approach was used by Chris Smith here[1] for similar changes
in arm pmu driver.
* In arm64 pmu driver: Got rid of unnecessary pointer typecastings.
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
Vinayak Kale (2):
genirq: Add an accessor for IRQ_PER_CPU flag
arm64: perf: add support for percpu pmu interrupt
arch/arm64/kernel/perf_event.c | 116 +++++++++++++++++++++++++++++-----------
include/linux/irqdesc.h | 8 +++
2 files changed, 94 insertions(+), 30 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V6 1/2] genirq: Add an accessor for IRQ_PER_CPU flag
2013-12-02 9:34 [PATCH V6 0/2] genirq: arm64: perf: support for percpu pmu interrupt Vinayak Kale
@ 2013-12-02 9:34 ` Vinayak Kale
2013-12-03 11:27 ` Will Deacon
2013-12-02 9:34 ` [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt Vinayak Kale
1 sibling, 1 reply; 9+ messages in thread
From: Vinayak Kale @ 2013-12-02 9:34 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds an accessor function for IRQ_PER_CPU flag.
The accessor function is useful to determine whether an IRQ is percpu or not.
This patch is based on an older patch posted by Chris Smith here [1].
There is a minor change w.r.t. Chris's original patch: The accessor function
is renamed as 'irq_is_percpu' instead of 'irq_is_per_cpu'.
[1]: http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
Signed-off-by: Chris Smith <chris.smith@st.com>
Signed-off-by: Vinayak Kale <vkale@apm.com>
---
include/linux/irqdesc.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 56fb646..26e2661 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -152,6 +152,14 @@ static inline int irq_balancing_disabled(unsigned int irq)
return desc->status_use_accessors & IRQ_NO_BALANCING_MASK;
}
+static inline int irq_is_percpu(unsigned int irq)
+{
+ struct irq_desc *desc;
+
+ desc = irq_to_desc(irq);
+ return desc->status_use_accessors & IRQ_PER_CPU;
+}
+
static inline void
irq_set_lockdep_class(unsigned int irq, struct lock_class_key *class)
{
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt
2013-12-02 9:34 [PATCH V6 0/2] genirq: arm64: perf: support for percpu pmu interrupt Vinayak Kale
2013-12-02 9:34 ` [PATCH V6 1/2] genirq: Add an accessor for IRQ_PER_CPU flag Vinayak Kale
@ 2013-12-02 9:34 ` Vinayak Kale
2013-12-03 11:30 ` Will Deacon
2013-12-03 13:50 ` Will Deacon
1 sibling, 2 replies; 9+ messages in thread
From: Vinayak Kale @ 2013-12-02 9:34 UTC (permalink / raw)
To: linux-arm-kernel
Add support for irq registration when pmu interrupt is percpu.
Signed-off-by: Vinayak Kale <vkale@apm.com>
Signed-off-by: Tuan Phan <tphan@apm.com>
---
arch/arm64/kernel/perf_event.c | 116 +++++++++++++++++++++++++++++-----------
1 file changed, 86 insertions(+), 30 deletions(-)
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cea1594..d2d562f 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -22,6 +22,7 @@
#include <linux/bitmap.h>
#include <linux/interrupt.h>
+#include <linux/irq.h>
#include <linux/kernel.h>
#include <linux/export.h>
#include <linux/perf_event.h>
@@ -363,26 +364,61 @@ validate_group(struct perf_event *event)
}
static void
+armpmu_disable_percpu_irq(void *data)
+{
+ struct arm_pmu *armpmu = data;
+ struct platform_device *pmu_device = armpmu->plat_device;
+ int irq = platform_get_irq(pmu_device, 0);
+
+ cpumask_test_and_clear_cpu(smp_processor_id(), &armpmu->active_irqs);
+ disable_percpu_irq(irq);
+}
+
+static void
armpmu_release_hardware(struct arm_pmu *armpmu)
{
- int i, irq, irqs;
+ int irq;
+ unsigned int i, irqs;
struct platform_device *pmu_device = armpmu->plat_device;
irqs = min(pmu_device->num_resources, num_possible_cpus());
+ if (!irqs)
+ return;
- for (i = 0; i < irqs; ++i) {
- if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
- continue;
- irq = platform_get_irq(pmu_device, i);
- if (irq >= 0)
- free_irq(irq, armpmu);
+ irq = platform_get_irq(pmu_device, 0);
+ if (irq <= 0)
+ return;
+
+ if (irq_is_percpu(irq)) {
+ on_each_cpu(armpmu_disable_percpu_irq, armpmu, 1);
+ free_percpu_irq(irq, &cpu_hw_events);
+ } else {
+ for (i = 0; i < irqs; ++i) {
+ if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
+ continue;
+ irq = platform_get_irq(pmu_device, i);
+ if (irq > 0)
+ free_irq(irq, armpmu);
+ }
}
}
+static void
+armpmu_enable_percpu_irq(void *data)
+{
+ struct arm_pmu *armpmu = data;
+ struct platform_device *pmu_device = armpmu->plat_device;
+ int irq = platform_get_irq(pmu_device, 0);
+
+ enable_percpu_irq(irq, IRQ_TYPE_NONE);
+ cpumask_set_cpu(smp_processor_id(), &armpmu->active_irqs);
+}
+
static int
armpmu_reserve_hardware(struct arm_pmu *armpmu)
{
- int i, err, irq, irqs;
+ int err, irq;
+ unsigned int i, irqs;
struct platform_device *pmu_device = armpmu->plat_device;
if (!pmu_device) {
@@ -391,39 +427,59 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
}
irqs = min(pmu_device->num_resources, num_possible_cpus());
- if (irqs < 1) {
+ if (!irqs) {
pr_err("no irqs for PMUs defined\n");
return -ENODEV;
}
- for (i = 0; i < irqs; ++i) {
- err = 0;
- irq = platform_get_irq(pmu_device, i);
- if (irq < 0)
- continue;
+ irq = platform_get_irq(pmu_device, 0);
+ if (irq <= 0) {
+ pr_err("failed to get valid irq for PMU device\n");
+ return -ENODEV;
+ }
- /*
- * If we have a single PMU interrupt that we can't shift,
- * assume that we're running on a uniprocessor machine and
- * continue. Otherwise, continue without this interrupt.
- */
- if (irq_set_affinity(irq, cpumask_of(i)) && irqs > 1) {
- pr_warning("unable to set irq affinity (irq=%d, cpu=%u)\n",
- irq, i);
- continue;
- }
+ if (irq_is_percpu(irq)) {
+ err = request_percpu_irq(irq, armpmu->handle_irq,
+ "arm-pmu", &cpu_hw_events);
- err = request_irq(irq, armpmu->handle_irq,
- IRQF_NOBALANCING,
- "arm-pmu", armpmu);
if (err) {
- pr_err("unable to request IRQ%d for ARM PMU counters\n",
- irq);
+ pr_err("unable to request percpu IRQ%d for ARM PMU counters\n",
+ irq);
armpmu_release_hardware(armpmu);
return err;
}
- cpumask_set_cpu(i, &armpmu->active_irqs);
+ on_each_cpu(armpmu_enable_percpu_irq, armpmu, 1);
+ } else {
+ for (i = 0; i < irqs; ++i) {
+ err = 0;
+ irq = platform_get_irq(pmu_device, i);
+ if (irq <= 0)
+ continue;
+
+ /*
+ * If we have a single PMU interrupt that we can't shift,
+ * assume that we're running on a uniprocessor machine and
+ * continue. Otherwise, continue without this interrupt.
+ */
+ if (irq_set_affinity(irq, cpumask_of(i)) && irqs > 1) {
+ pr_warning("unable to set irq affinity (irq=%d, cpu=%u)\n",
+ irq, i);
+ continue;
+ }
+
+ err = request_irq(irq, armpmu->handle_irq,
+ IRQF_NOBALANCING,
+ "arm-pmu", armpmu);
+ if (err) {
+ pr_err("unable to request IRQ%d for ARM PMU counters\n",
+ irq);
+ armpmu_release_hardware(armpmu);
+ return err;
+ }
+
+ cpumask_set_cpu(i, &armpmu->active_irqs);
+ }
}
return 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V6 1/2] genirq: Add an accessor for IRQ_PER_CPU flag
2013-12-02 9:34 ` [PATCH V6 1/2] genirq: Add an accessor for IRQ_PER_CPU flag Vinayak Kale
@ 2013-12-03 11:27 ` Will Deacon
0 siblings, 0 replies; 9+ messages in thread
From: Will Deacon @ 2013-12-03 11:27 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 02, 2013 at 09:34:02AM +0000, Vinayak Kale wrote:
> This patch adds an accessor function for IRQ_PER_CPU flag.
> The accessor function is useful to determine whether an IRQ is percpu or not.
>
> This patch is based on an older patch posted by Chris Smith here [1].
> There is a minor change w.r.t. Chris's original patch: The accessor function
> is renamed as 'irq_is_percpu' instead of 'irq_is_per_cpu'.
>
> [1]: http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
>
> Signed-off-by: Chris Smith <chris.smith@st.com>
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> ---
> include/linux/irqdesc.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index 56fb646..26e2661 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -152,6 +152,14 @@ static inline int irq_balancing_disabled(unsigned int irq)
> return desc->status_use_accessors & IRQ_NO_BALANCING_MASK;
> }
>
> +static inline int irq_is_percpu(unsigned int irq)
> +{
> + struct irq_desc *desc;
> +
> + desc = irq_to_desc(irq);
> + return desc->status_use_accessors & IRQ_PER_CPU;
> +}
> +
> static inline void
> irq_set_lockdep_class(unsigned int irq, struct lock_class_key *class)
> {
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt
2013-12-02 9:34 ` [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt Vinayak Kale
@ 2013-12-03 11:30 ` Will Deacon
2013-12-03 11:49 ` Vinayak Kale
2013-12-03 13:50 ` Will Deacon
1 sibling, 1 reply; 9+ messages in thread
From: Will Deacon @ 2013-12-03 11:30 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 02, 2013 at 09:34:03AM +0000, Vinayak Kale wrote:
> Add support for irq registration when pmu interrupt is percpu.
>
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> Signed-off-by: Tuan Phan <tphan@apm.com>
> ---
> arch/arm64/kernel/perf_event.c | 116 +++++++++++++++++++++++++++++-----------
> 1 file changed, 86 insertions(+), 30 deletions(-)
>
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cea1594..d2d562f 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -22,6 +22,7 @@
>
> #include <linux/bitmap.h>
> #include <linux/interrupt.h>
> +#include <linux/irq.h>
> #include <linux/kernel.h>
> #include <linux/export.h>
> #include <linux/perf_event.h>
> @@ -363,26 +364,61 @@ validate_group(struct perf_event *event)
> }
>
> static void
> +armpmu_disable_percpu_irq(void *data)
> +{
> + struct arm_pmu *armpmu = data;
> + struct platform_device *pmu_device = armpmu->plat_device;
> + int irq = platform_get_irq(pmu_device, 0);
> +
> + cpumask_test_and_clear_cpu(smp_processor_id(), &armpmu->active_irqs);
> + disable_percpu_irq(irq);
> +}
> +
> +static void
> armpmu_release_hardware(struct arm_pmu *armpmu)
> {
> - int i, irq, irqs;
> + int irq;
Why did you not make this unsigned, like I suggested?
> + unsigned int i, irqs;
> struct platform_device *pmu_device = armpmu->plat_device;
>
> irqs = min(pmu_device->num_resources, num_possible_cpus());
> + if (!irqs)
> + return;
>
> - for (i = 0; i < irqs; ++i) {
> - if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
> - continue;
> - irq = platform_get_irq(pmu_device, i);
> - if (irq >= 0)
> - free_irq(irq, armpmu);
> + irq = platform_get_irq(pmu_device, 0);
> + if (irq <= 0)
> + return;
Then this is just an if (!irq), as I mentioned last time.
Will
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt
2013-12-03 11:30 ` Will Deacon
@ 2013-12-03 11:49 ` Vinayak Kale
2013-12-03 13:41 ` Will Deacon
0 siblings, 1 reply; 9+ messages in thread
From: Vinayak Kale @ 2013-12-03 11:49 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 3, 2013 at 5:00 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Dec 02, 2013 at 09:34:03AM +0000, Vinayak Kale wrote:
>> Add support for irq registration when pmu interrupt is percpu.
>>
>> Signed-off-by: Vinayak Kale <vkale@apm.com>
>> Signed-off-by: Tuan Phan <tphan@apm.com>
>> ---
>> arch/arm64/kernel/perf_event.c | 116 +++++++++++++++++++++++++++++-----------
>> 1 file changed, 86 insertions(+), 30 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index cea1594..d2d562f 100644
>> --- a/arch/arm64/kernel/perf_event.c
>> +++ b/arch/arm64/kernel/perf_event.c
>> @@ -22,6 +22,7 @@
>>
>> #include <linux/bitmap.h>
>> #include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> #include <linux/kernel.h>
>> #include <linux/export.h>
>> #include <linux/perf_event.h>
>> @@ -363,26 +364,61 @@ validate_group(struct perf_event *event)
>> }
>>
>> static void
>> +armpmu_disable_percpu_irq(void *data)
>> +{
>> + struct arm_pmu *armpmu = data;
>> + struct platform_device *pmu_device = armpmu->plat_device;
>> + int irq = platform_get_irq(pmu_device, 0);
>> +
>> + cpumask_test_and_clear_cpu(smp_processor_id(), &armpmu->active_irqs);
>> + disable_percpu_irq(irq);
>> +}
>> +
>> +static void
>> armpmu_release_hardware(struct arm_pmu *armpmu)
>> {
>> - int i, irq, irqs;
>> + int irq;
>
> Why did you not make this unsigned, like I suggested?
Suggestion was to make 'irqs' variable unsigned and modify the check
for 'irqs' to if (!irqs).
This patch incorporates that suggestion.
We have to keep 'irq' signed only. 'platform_get_irq()' can return error value.
>
>> + unsigned int i, irqs;
>> struct platform_device *pmu_device = armpmu->plat_device;
>>
>> irqs = min(pmu_device->num_resources, num_possible_cpus());
>> + if (!irqs)
>> + return;
>>
>> - for (i = 0; i < irqs; ++i) {
>> - if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
>> - continue;
>> - irq = platform_get_irq(pmu_device, i);
>> - if (irq >= 0)
>> - free_irq(irq, armpmu);
>> + irq = platform_get_irq(pmu_device, 0);
>> + if (irq <= 0)
>> + return;
>
> Then this is just an if (!irq), as I mentioned last time.
Please see my above comment.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt
2013-12-03 11:49 ` Vinayak Kale
@ 2013-12-03 13:41 ` Will Deacon
0 siblings, 0 replies; 9+ messages in thread
From: Will Deacon @ 2013-12-03 13:41 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 03, 2013 at 11:49:11AM +0000, Vinayak Kale wrote:
> On Tue, Dec 3, 2013 at 5:00 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Dec 02, 2013 at 09:34:03AM +0000, Vinayak Kale wrote:
> >> Add support for irq registration when pmu interrupt is percpu.
> >>
> >> Signed-off-by: Vinayak Kale <vkale@apm.com>
> >> Signed-off-by: Tuan Phan <tphan@apm.com>
> >> ---
> >> arch/arm64/kernel/perf_event.c | 116 +++++++++++++++++++++++++++++-----------
> >> 1 file changed, 86 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> >> index cea1594..d2d562f 100644
> >> --- a/arch/arm64/kernel/perf_event.c
> >> +++ b/arch/arm64/kernel/perf_event.c
> >> @@ -22,6 +22,7 @@
> >>
> >> #include <linux/bitmap.h>
> >> #include <linux/interrupt.h>
> >> +#include <linux/irq.h>
> >> #include <linux/kernel.h>
> >> #include <linux/export.h>
> >> #include <linux/perf_event.h>
> >> @@ -363,26 +364,61 @@ validate_group(struct perf_event *event)
> >> }
> >>
> >> static void
> >> +armpmu_disable_percpu_irq(void *data)
> >> +{
> >> + struct arm_pmu *armpmu = data;
> >> + struct platform_device *pmu_device = armpmu->plat_device;
> >> + int irq = platform_get_irq(pmu_device, 0);
> >> +
> >> + cpumask_test_and_clear_cpu(smp_processor_id(), &armpmu->active_irqs);
> >> + disable_percpu_irq(irq);
> >> +}
> >> +
> >> +static void
> >> armpmu_release_hardware(struct arm_pmu *armpmu)
> >> {
> >> - int i, irq, irqs;
> >> + int irq;
> >
> > Why did you not make this unsigned, like I suggested?
>
> Suggestion was to make 'irqs' variable unsigned and modify the check
> for 'irqs' to if (!irqs).
> This patch incorporates that suggestion.
>
> We have to keep 'irq' signed only. 'platform_get_irq()' can return error value.
Damn, yes, I see the issue there. Ok, I'll go back and take another look at
your patch...
Will
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt
2013-12-02 9:34 ` [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt Vinayak Kale
2013-12-03 11:30 ` Will Deacon
@ 2013-12-03 13:50 ` Will Deacon
2013-12-04 6:26 ` Vinayak Kale
1 sibling, 1 reply; 9+ messages in thread
From: Will Deacon @ 2013-12-03 13:50 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 02, 2013 at 09:34:03AM +0000, Vinayak Kale wrote:
> static void
> +armpmu_disable_percpu_irq(void *data)
> +{
> + struct arm_pmu *armpmu = data;
> + struct platform_device *pmu_device = armpmu->plat_device;
> + int irq = platform_get_irq(pmu_device, 0);
> +
> + cpumask_test_and_clear_cpu(smp_processor_id(), &armpmu->active_irqs);
Why not just cpumask_clear_cpu?
> + disable_percpu_irq(irq);
> +}
> +
> +static void
> armpmu_release_hardware(struct arm_pmu *armpmu)
> {
> - int i, irq, irqs;
> + int irq;
> + unsigned int i, irqs;
> struct platform_device *pmu_device = armpmu->plat_device;
>
> irqs = min(pmu_device->num_resources, num_possible_cpus());
> + if (!irqs)
> + return;
>
> - for (i = 0; i < irqs; ++i) {
> - if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
> - continue;
> - irq = platform_get_irq(pmu_device, i);
> - if (irq >= 0)
> - free_irq(irq, armpmu);
> + irq = platform_get_irq(pmu_device, 0);
> + if (irq <= 0)
> + return;
> +
> + if (irq_is_percpu(irq)) {
> + on_each_cpu(armpmu_disable_percpu_irq, armpmu, 1);
> + free_percpu_irq(irq, &cpu_hw_events);
> + } else {
> + for (i = 0; i < irqs; ++i) {
> + if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
> + continue;
> + irq = platform_get_irq(pmu_device, i);
> + if (irq > 0)
> + free_irq(irq, armpmu);
> + }
> }
> }
>
> +static void
> +armpmu_enable_percpu_irq(void *data)
> +{
> + struct arm_pmu *armpmu = data;
> + struct platform_device *pmu_device = armpmu->plat_device;
> + int irq = platform_get_irq(pmu_device, 0);
> +
> + enable_percpu_irq(irq, IRQ_TYPE_NONE);
> + cpumask_set_cpu(smp_processor_id(), &armpmu->active_irqs);
Hmm, wouldn't it make more sense to pass the irq in data, then deal with the
mask in the caller? (since the mask will *always* be updated by each CPU).
Similarly for the disable path.
Will
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt
2013-12-03 13:50 ` Will Deacon
@ 2013-12-04 6:26 ` Vinayak Kale
0 siblings, 0 replies; 9+ messages in thread
From: Vinayak Kale @ 2013-12-04 6:26 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 3, 2013 at 7:20 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Dec 02, 2013 at 09:34:03AM +0000, Vinayak Kale wrote:
>> static void
>> +armpmu_disable_percpu_irq(void *data)
>> +{
>> + struct arm_pmu *armpmu = data;
>> + struct platform_device *pmu_device = armpmu->plat_device;
>> + int irq = platform_get_irq(pmu_device, 0);
>> +
>> + cpumask_test_and_clear_cpu(smp_processor_id(), &armpmu->active_irqs);
>
> Why not just cpumask_clear_cpu?
Yes, that would have serve the purpose. It was due to dumb copy/paste
from non-percpu counterpart.
>
>> + disable_percpu_irq(irq);
>> +}
>> +
>> +static void
>> armpmu_release_hardware(struct arm_pmu *armpmu)
>> {
>> - int i, irq, irqs;
>> + int irq;
>> + unsigned int i, irqs;
>> struct platform_device *pmu_device = armpmu->plat_device;
>>
>> irqs = min(pmu_device->num_resources, num_possible_cpus());
>> + if (!irqs)
>> + return;
>>
>> - for (i = 0; i < irqs; ++i) {
>> - if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
>> - continue;
>> - irq = platform_get_irq(pmu_device, i);
>> - if (irq >= 0)
>> - free_irq(irq, armpmu);
>> + irq = platform_get_irq(pmu_device, 0);
>> + if (irq <= 0)
>> + return;
>> +
>> + if (irq_is_percpu(irq)) {
>> + on_each_cpu(armpmu_disable_percpu_irq, armpmu, 1);
>> + free_percpu_irq(irq, &cpu_hw_events);
>> + } else {
>> + for (i = 0; i < irqs; ++i) {
>> + if (!cpumask_test_and_clear_cpu(i, &armpmu->active_irqs))
>> + continue;
>> + irq = platform_get_irq(pmu_device, i);
>> + if (irq > 0)
>> + free_irq(irq, armpmu);
>> + }
>> }
>> }
>>
>> +static void
>> +armpmu_enable_percpu_irq(void *data)
>> +{
>> + struct arm_pmu *armpmu = data;
>> + struct platform_device *pmu_device = armpmu->plat_device;
>> + int irq = platform_get_irq(pmu_device, 0);
>> +
>> + enable_percpu_irq(irq, IRQ_TYPE_NONE);
>> + cpumask_set_cpu(smp_processor_id(), &armpmu->active_irqs);
>
> Hmm, wouldn't it make more sense to pass the irq in data, then deal with the
> mask in the caller? (since the mask will *always* be updated by each CPU).
>
> Similarly for the disable path.
Okay.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-12-04 6:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-02 9:34 [PATCH V6 0/2] genirq: arm64: perf: support for percpu pmu interrupt Vinayak Kale
2013-12-02 9:34 ` [PATCH V6 1/2] genirq: Add an accessor for IRQ_PER_CPU flag Vinayak Kale
2013-12-03 11:27 ` Will Deacon
2013-12-02 9:34 ` [PATCH V6 2/2] arm64: perf: add support for percpu pmu interrupt Vinayak Kale
2013-12-03 11:30 ` Will Deacon
2013-12-03 11:49 ` Vinayak Kale
2013-12-03 13:41 ` Will Deacon
2013-12-03 13:50 ` Will Deacon
2013-12-04 6:26 ` Vinayak Kale
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).