On Tue, Dec 3, 2013 at 5:00 PM, Will Deacon [off-list ref] wrote:

On Mon, Dec 02, 2013 at 09:34:03AM +0000, Vinayak Kale wrote:

quoted

Add support for irq registration when pmu interrupt is percpu.

Signed-off-by: Vinayak Kale <redacted>
Signed-off-by: Tuan Phan <redacted>
---
 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.

quoted

+     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.