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

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

quoted

 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.

quoted

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