All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-11-06 12:07 ` Vinayak Kale
  0 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-06 12:07 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: tglx, will.deacon, patches, jcm, Vinayak Kale

This patch series adds support to handle interrupt registration/deregistration
in arm64 pmu driver when pmu interrupt type is percpu.

Patches in this patch series were previously sent out as separate patches [1].
This patch series incorporates comments/fixes suggested for original patches.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html 

Vinayak Kale (2):
  genirq: error reporting in request_percpu_irq() and
    request_threaded_irq()
  arm64: perf: add support for percpu pmu interrupt

 arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
 kernel/irq/manage.c            |   12 +++--
 2 files changed, 89 insertions(+), 32 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-11-06 12:07 ` Vinayak Kale
  0 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-06 12:07 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.

Patches in this patch series were previously sent out as separate patches [1].
This patch series incorporates comments/fixes suggested for original patches.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html 

Vinayak Kale (2):
  genirq: error reporting in request_percpu_irq() and
    request_threaded_irq()
  arm64: perf: add support for percpu pmu interrupt

 arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
 kernel/irq/manage.c            |   12 +++--
 2 files changed, 89 insertions(+), 32 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/2] genirq: error reporting in request_percpu_irq() and request_threaded_irq()
  2013-11-06 12:07 ` Vinayak Kale
@ 2013-11-06 12:07   ` Vinayak Kale
  -1 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-06 12:07 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: tglx, will.deacon, patches, jcm, Vinayak Kale

Return a separate error code when invalid interrupt type is passed to
request_percpu_irq() and request_threaded_irq().

Suggested-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Vinayak Kale <vkale@apm.com>
---
 kernel/irq/manage.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 514bcfd..08fffaf 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1414,10 +1414,12 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	if (!desc)
 		return -EINVAL;
 
-	if (!irq_settings_can_request(desc) ||
-	    WARN_ON(irq_settings_is_per_cpu_devid(desc)))
+	if (!irq_settings_can_request(desc))
 		return -EINVAL;
 
+	if (irq_settings_is_per_cpu_devid(desc))
+		return -EPERM;
+
 	if (!handler) {
 		if (!thread_fn)
 			return -EINVAL;
@@ -1671,10 +1673,12 @@ int request_percpu_irq(unsigned int irq, irq_handler_t handler,
 		return -EINVAL;
 
 	desc = irq_to_desc(irq);
-	if (!desc || !irq_settings_can_request(desc) ||
-	    !irq_settings_is_per_cpu_devid(desc))
+	if (!desc || !irq_settings_can_request(desc))
 		return -EINVAL;
 
+	if (!irq_settings_is_per_cpu_devid(desc))
+		return -EPERM;
+
 	action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);
 	if (!action)
 		return -ENOMEM;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 1/2] genirq: error reporting in request_percpu_irq() and request_threaded_irq()
@ 2013-11-06 12:07   ` Vinayak Kale
  0 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-06 12:07 UTC (permalink / raw)
  To: linux-arm-kernel

Return a separate error code when invalid interrupt type is passed to
request_percpu_irq() and request_threaded_irq().

Suggested-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Vinayak Kale <vkale@apm.com>
---
 kernel/irq/manage.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 514bcfd..08fffaf 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1414,10 +1414,12 @@ int request_threaded_irq(unsigned int irq, irq_handler_t handler,
 	if (!desc)
 		return -EINVAL;
 
-	if (!irq_settings_can_request(desc) ||
-	    WARN_ON(irq_settings_is_per_cpu_devid(desc)))
+	if (!irq_settings_can_request(desc))
 		return -EINVAL;
 
+	if (irq_settings_is_per_cpu_devid(desc))
+		return -EPERM;
+
 	if (!handler) {
 		if (!thread_fn)
 			return -EINVAL;
@@ -1671,10 +1673,12 @@ int request_percpu_irq(unsigned int irq, irq_handler_t handler,
 		return -EINVAL;
 
 	desc = irq_to_desc(irq);
-	if (!desc || !irq_settings_can_request(desc) ||
-	    !irq_settings_is_per_cpu_devid(desc))
+	if (!desc || !irq_settings_can_request(desc))
 		return -EINVAL;
 
+	if (!irq_settings_is_per_cpu_devid(desc))
+		return -EPERM;
+
 	action = kzalloc(sizeof(struct irqaction), GFP_KERNEL);
 	if (!action)
 		return -ENOMEM;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt
  2013-11-06 12:07 ` Vinayak Kale
@ 2013-11-06 12:07   ` Vinayak Kale
  -1 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-06 12:07 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel
  Cc: tglx, will.deacon, patches, jcm, Vinayak Kale, Tuan Phan

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 |  109 +++++++++++++++++++++++++++++-----------
 1 file changed, 81 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cea1594..e60dae3 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -44,6 +44,7 @@
 static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
 static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
 static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
+static bool is_percpuirq;
 
 #define to_arm_pmu(p) (container_of(p, struct arm_pmu, pmu))
 
@@ -363,22 +364,51 @@ validate_group(struct perf_event *event)
 }
 
 static void
+armpmu_disable_percpu_irq(void *data)
+{
+	struct arm_pmu *armpmu = (struct arm_pmu *)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;
 	struct platform_device *pmu_device = armpmu->plat_device;
 
-	irqs = min(pmu_device->num_resources, num_possible_cpus());
+	irq = platform_get_irq(pmu_device, 0);
 
-	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);
+	if (is_percpuirq) {
+		on_each_cpu(armpmu_disable_percpu_irq, (void *)armpmu, 1);
+		free_percpu_irq(irq, &cpu_hw_events);
+	} else {
+		irqs = min(pmu_device->num_resources, num_possible_cpus());
+
+		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 = (struct arm_pmu *)data;
+	struct platform_device *pmu_device = armpmu->plat_device;
+	int irq = platform_get_irq(pmu_device, 0);
+
+	enable_percpu_irq(irq, 0);
+	cpumask_set_cpu(smp_processor_id(), &armpmu->active_irqs);
+}
+
 static int
 armpmu_reserve_hardware(struct arm_pmu *armpmu)
 {
@@ -396,34 +426,57 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
 		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 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;
-		}
+	/*
+	 * Try 'request_percpu_irq' first. If it's return value indicates
+	 * that irq is not percpu then try 'request_irq'
+	 */
+	err = request_percpu_irq(irq, armpmu->handle_irq,
+			"arm-pmu", &cpu_hw_events);
+	if (err != -EPERM) {
+		is_percpuirq = true;
 
-		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, (void *)armpmu, 1);
+	} else {
+		is_percpuirq = false;
+
+		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] 24+ messages in thread

* [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt
@ 2013-11-06 12:07   ` Vinayak Kale
  0 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-06 12:07 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 |  109 +++++++++++++++++++++++++++++-----------
 1 file changed, 81 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index cea1594..e60dae3 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -44,6 +44,7 @@
 static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
 static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
 static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
+static bool is_percpuirq;
 
 #define to_arm_pmu(p) (container_of(p, struct arm_pmu, pmu))
 
@@ -363,22 +364,51 @@ validate_group(struct perf_event *event)
 }
 
 static void
+armpmu_disable_percpu_irq(void *data)
+{
+	struct arm_pmu *armpmu = (struct arm_pmu *)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;
 	struct platform_device *pmu_device = armpmu->plat_device;
 
-	irqs = min(pmu_device->num_resources, num_possible_cpus());
+	irq = platform_get_irq(pmu_device, 0);
 
-	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);
+	if (is_percpuirq) {
+		on_each_cpu(armpmu_disable_percpu_irq, (void *)armpmu, 1);
+		free_percpu_irq(irq, &cpu_hw_events);
+	} else {
+		irqs = min(pmu_device->num_resources, num_possible_cpus());
+
+		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 = (struct arm_pmu *)data;
+	struct platform_device *pmu_device = armpmu->plat_device;
+	int irq = platform_get_irq(pmu_device, 0);
+
+	enable_percpu_irq(irq, 0);
+	cpumask_set_cpu(smp_processor_id(), &armpmu->active_irqs);
+}
+
 static int
 armpmu_reserve_hardware(struct arm_pmu *armpmu)
 {
@@ -396,34 +426,57 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
 		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 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;
-		}
+	/*
+	 * Try 'request_percpu_irq' first. If it's return value indicates
+	 * that irq is not percpu then try 'request_irq'
+	 */
+	err = request_percpu_irq(irq, armpmu->handle_irq,
+			"arm-pmu", &cpu_hw_events);
+	if (err != -EPERM) {
+		is_percpuirq = true;
 
-		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, (void *)armpmu, 1);
+	} else {
+		is_percpuirq = false;
+
+		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] 24+ messages in thread

* Re: [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt
  2013-11-06 12:07   ` Vinayak Kale
@ 2013-11-08 15:57     ` Will Deacon
  -1 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2013-11-08 15:57 UTC (permalink / raw)
  To: Vinayak Kale
  Cc: linux-kernel, linux-arm-kernel, tglx, patches, jcm, Tuan Phan

On Wed, Nov 06, 2013 at 12:07:37PM +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 |  109 +++++++++++++++++++++++++++++-----------
>  1 file changed, 81 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cea1594..e60dae3 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -44,6 +44,7 @@
>  static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
>  static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
>  static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
> +static bool is_percpuirq;

This isn't a global property; it should be part of the struct arm_pmu.
The rest of the patch looks ok, but obvious requires you to merge the core
changes first.

Will

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt
@ 2013-11-08 15:57     ` Will Deacon
  0 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2013-11-08 15:57 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 06, 2013 at 12:07:37PM +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 |  109 +++++++++++++++++++++++++++++-----------
>  1 file changed, 81 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cea1594..e60dae3 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -44,6 +44,7 @@
>  static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
>  static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
>  static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
> +static bool is_percpuirq;

This isn't a global property; it should be part of the struct arm_pmu.
The rest of the patch looks ok, but obvious requires you to merge the core
changes first.

Will

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
  2013-11-06 12:07 ` Vinayak Kale
@ 2013-11-09  1:04   ` Stephen Boyd
  -1 siblings, 0 replies; 24+ messages in thread
From: Stephen Boyd @ 2013-11-09  1:04 UTC (permalink / raw)
  To: Vinayak Kale, linux-kernel, linux-arm-kernel
  Cc: patches, tglx, will.deacon, jcm

On 11/06/13 04:07, Vinayak Kale wrote:
> This patch series adds support to handle interrupt registration/deregistration
> in arm64 pmu driver when pmu interrupt type is percpu.
>
> Patches in this patch series were previously sent out as separate patches [1].
> This patch series incorporates comments/fixes suggested for original patches.
>
> [1] 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html 
>
> Vinayak Kale (2):
>   genirq: error reporting in request_percpu_irq() and
>     request_threaded_irq()
>   arm64: perf: add support for percpu pmu interrupt
>
>  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
>  kernel/irq/manage.c            |   12 +++--
>  2 files changed, 89 insertions(+), 32 deletions(-)
>

What ever happened to the approach here[1]? It doesn't look very nice to
have to request the irq first as a per-cpu interrupt and then try as a
non-percpu interrupt when genirq already knows if its per-cpu or not.

[1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-11-09  1:04   ` Stephen Boyd
  0 siblings, 0 replies; 24+ messages in thread
From: Stephen Boyd @ 2013-11-09  1:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/06/13 04:07, Vinayak Kale wrote:
> This patch series adds support to handle interrupt registration/deregistration
> in arm64 pmu driver when pmu interrupt type is percpu.
>
> Patches in this patch series were previously sent out as separate patches [1].
> This patch series incorporates comments/fixes suggested for original patches.
>
> [1] 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html 
>
> Vinayak Kale (2):
>   genirq: error reporting in request_percpu_irq() and
>     request_threaded_irq()
>   arm64: perf: add support for percpu pmu interrupt
>
>  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
>  kernel/irq/manage.c            |   12 +++--
>  2 files changed, 89 insertions(+), 32 deletions(-)
>

What ever happened to the approach here[1]? It doesn't look very nice to
have to request the irq first as a per-cpu interrupt and then try as a
non-percpu interrupt when genirq already knows if its per-cpu or not.

[1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt
  2013-11-06 12:07   ` Vinayak Kale
@ 2013-11-09  1:04     ` Stephen Boyd
  -1 siblings, 0 replies; 24+ messages in thread
From: Stephen Boyd @ 2013-11-09  1:04 UTC (permalink / raw)
  To: Vinayak Kale, linux-kernel, linux-arm-kernel
  Cc: will.deacon, patches, jcm, tglx, Tuan Phan

Just some pointer casting nits.

On 11/06/13 04:07, 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 |  109 +++++++++++++++++++++++++++++-----------
>  1 file changed, 81 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cea1594..e60dae3 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -44,6 +44,7 @@
>  static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
>  static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
>  static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
> +static bool is_percpuirq;
>  
>  #define to_arm_pmu(p) (container_of(p, struct arm_pmu, pmu))
>  
> @@ -363,22 +364,51 @@ validate_group(struct perf_event *event)
>  }
>  
>  static void
> +armpmu_disable_percpu_irq(void *data)
> +{
> +	struct arm_pmu *armpmu = (struct arm_pmu *)data;

Useless cast from void *.

> +	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_enable_percpu_irq(void *data)
> +{
> +	struct arm_pmu *armpmu = (struct arm_pmu *)data;

Here too.

> +	struct platform_device *pmu_device = armpmu->plat_device;
> +	int irq = platform_get_irq(pmu_device, 0);
> +
> +	enable_percpu_irq(irq, 0);
> +	cpumask_set_cpu(smp_processor_id(), &armpmu->active_irqs);
> +}
> +
>  static int
>  armpmu_reserve_hardware(struct arm_pmu *armpmu)
>  {
> @@ -396,34 +426,57 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
>  		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 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;
> -		}
> +	/*
> +	 * Try 'request_percpu_irq' first. If it's return value indicates
> +	 * that irq is not percpu then try 'request_irq'
> +	 */
> +	err = request_percpu_irq(irq, armpmu->handle_irq,
> +			"arm-pmu", &cpu_hw_events);
> +	if (err != -EPERM) {
> +		is_percpuirq = true;
>  
> -		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, (void *)armpmu, 1);

Looks like a useless cast to void * here.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt
@ 2013-11-09  1:04     ` Stephen Boyd
  0 siblings, 0 replies; 24+ messages in thread
From: Stephen Boyd @ 2013-11-09  1:04 UTC (permalink / raw)
  To: linux-arm-kernel

Just some pointer casting nits.

On 11/06/13 04:07, 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 |  109 +++++++++++++++++++++++++++++-----------
>  1 file changed, 81 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cea1594..e60dae3 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -44,6 +44,7 @@
>  static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
>  static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
>  static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
> +static bool is_percpuirq;
>  
>  #define to_arm_pmu(p) (container_of(p, struct arm_pmu, pmu))
>  
> @@ -363,22 +364,51 @@ validate_group(struct perf_event *event)
>  }
>  
>  static void
> +armpmu_disable_percpu_irq(void *data)
> +{
> +	struct arm_pmu *armpmu = (struct arm_pmu *)data;

Useless cast from void *.

> +	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_enable_percpu_irq(void *data)
> +{
> +	struct arm_pmu *armpmu = (struct arm_pmu *)data;

Here too.

> +	struct platform_device *pmu_device = armpmu->plat_device;
> +	int irq = platform_get_irq(pmu_device, 0);
> +
> +	enable_percpu_irq(irq, 0);
> +	cpumask_set_cpu(smp_processor_id(), &armpmu->active_irqs);
> +}
> +
>  static int
>  armpmu_reserve_hardware(struct arm_pmu *armpmu)
>  {
> @@ -396,34 +426,57 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
>  		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 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;
> -		}
> +	/*
> +	 * Try 'request_percpu_irq' first. If it's return value indicates
> +	 * that irq is not percpu then try 'request_irq'
> +	 */
> +	err = request_percpu_irq(irq, armpmu->handle_irq,
> +			"arm-pmu", &cpu_hw_events);
> +	if (err != -EPERM) {
> +		is_percpuirq = true;
>  
> -		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, (void *)armpmu, 1);

Looks like a useless cast to void * here.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
  2013-11-09  1:04   ` Stephen Boyd
@ 2013-11-11  9:25     ` Vinayak Kale
  -1 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-11  9:25 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-kernel, linux-arm-kernel, patches, tglx, Will Deacon, jcm

On Sat, Nov 9, 2013 at 6:34 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/06/13 04:07, Vinayak Kale wrote:
>> This patch series adds support to handle interrupt registration/deregistration
>> in arm64 pmu driver when pmu interrupt type is percpu.
>>
>> Patches in this patch series were previously sent out as separate patches [1].
>> This patch series incorporates comments/fixes suggested for original patches.
>>
>> [1]
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html
>>
>> Vinayak Kale (2):
>>   genirq: error reporting in request_percpu_irq() and
>>     request_threaded_irq()
>>   arm64: perf: add support for percpu pmu interrupt
>>
>>  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
>>  kernel/irq/manage.c            |   12 +++--
>>  2 files changed, 89 insertions(+), 32 deletions(-)
>>
>
> What ever happened to the approach here[1]? It doesn't look very nice to
> have to request the irq first as a per-cpu interrupt and then try as a
> non-percpu interrupt when genirq already knows if its per-cpu or not.
>
> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
I don't see any discussions on mailing list on approach taken in above patch.

IMO, if fixing up minor error reporting in existing functions can do
the job then we should avoid adding a new function.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-11-11  9:25     ` Vinayak Kale
  0 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-11  9:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Nov 9, 2013 at 6:34 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/06/13 04:07, Vinayak Kale wrote:
>> This patch series adds support to handle interrupt registration/deregistration
>> in arm64 pmu driver when pmu interrupt type is percpu.
>>
>> Patches in this patch series were previously sent out as separate patches [1].
>> This patch series incorporates comments/fixes suggested for original patches.
>>
>> [1]
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html
>>
>> Vinayak Kale (2):
>>   genirq: error reporting in request_percpu_irq() and
>>     request_threaded_irq()
>>   arm64: perf: add support for percpu pmu interrupt
>>
>>  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
>>  kernel/irq/manage.c            |   12 +++--
>>  2 files changed, 89 insertions(+), 32 deletions(-)
>>
>
> What ever happened to the approach here[1]? It doesn't look very nice to
> have to request the irq first as a per-cpu interrupt and then try as a
> non-percpu interrupt when genirq already knows if its per-cpu or not.
>
> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
I don't see any discussions on mailing list on approach taken in above patch.

IMO, if fixing up minor error reporting in existing functions can do
the job then we should avoid adding a new function.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt
  2013-11-08 15:57     ` Will Deacon
@ 2013-11-11  9:30       ` Vinayak Kale
  -1 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-11  9:30 UTC (permalink / raw)
  To: Will Deacon; +Cc: linux-kernel, linux-arm-kernel, tglx, patches, jcm, Tuan Phan

On Fri, Nov 8, 2013 at 9:27 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Nov 06, 2013 at 12:07:37PM +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 |  109 +++++++++++++++++++++++++++++-----------
>>  1 file changed, 81 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index cea1594..e60dae3 100644
>> --- a/arch/arm64/kernel/perf_event.c
>> +++ b/arch/arm64/kernel/perf_event.c
>> @@ -44,6 +44,7 @@
>>  static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
>>  static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
>>  static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
>> +static bool is_percpuirq;
>
> This isn't a global property; it should be part of the struct arm_pmu.
> The rest of the patch looks ok, but obvious requires you to merge the core
> changes first.

Okay. I will incorporate above change in next revision once the core
changes are agreed upon.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt
@ 2013-11-11  9:30       ` Vinayak Kale
  0 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-11  9:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Nov 8, 2013 at 9:27 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Nov 06, 2013 at 12:07:37PM +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 |  109 +++++++++++++++++++++++++++++-----------
>>  1 file changed, 81 insertions(+), 28 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
>> index cea1594..e60dae3 100644
>> --- a/arch/arm64/kernel/perf_event.c
>> +++ b/arch/arm64/kernel/perf_event.c
>> @@ -44,6 +44,7 @@
>>  static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
>>  static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
>>  static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
>> +static bool is_percpuirq;
>
> This isn't a global property; it should be part of the struct arm_pmu.
> The rest of the patch looks ok, but obvious requires you to merge the core
> changes first.

Okay. I will incorporate above change in next revision once the core
changes are agreed upon.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
  2013-11-09  1:04   ` Stephen Boyd
@ 2013-11-11 10:44     ` Will Deacon
  -1 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2013-11-11 10:44 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Vinayak Kale, linux-kernel, linux-arm-kernel, patches, tglx, jcm

On Sat, Nov 09, 2013 at 01:04:23AM +0000, Stephen Boyd wrote:
> On 11/06/13 04:07, Vinayak Kale wrote:
> > This patch series adds support to handle interrupt registration/deregistration
> > in arm64 pmu driver when pmu interrupt type is percpu.
> >
> > Patches in this patch series were previously sent out as separate patches [1].
> > This patch series incorporates comments/fixes suggested for original patches.
> >
> > [1] 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html 
> >
> > Vinayak Kale (2):
> >   genirq: error reporting in request_percpu_irq() and
> >     request_threaded_irq()
> >   arm64: perf: add support for percpu pmu interrupt
> >
> >  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
> >  kernel/irq/manage.c            |   12 +++--
> >  2 files changed, 89 insertions(+), 32 deletions(-)
> >
> 
> What ever happened to the approach here[1]? It doesn't look very nice to
> have to request the irq first as a per-cpu interrupt and then try as a
> non-percpu interrupt when genirq already knows if its per-cpu or not.
> 
> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html

Hmm, I'd completely forgotten about that approach. Whilst it certainly looks
cleaner from a user perspective, I always get scared when I see
'desc->status_use_accessors' since it tends to incur the wrath of tglx :)

That said, I guess that should be fine in irqdesc.h (basically adding a new
accessor). Chris went missing after sending those initial patches, so
perhaps Vinayak could look at resurrecting those?

Will

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-11-11 10:44     ` Will Deacon
  0 siblings, 0 replies; 24+ messages in thread
From: Will Deacon @ 2013-11-11 10:44 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Nov 09, 2013 at 01:04:23AM +0000, Stephen Boyd wrote:
> On 11/06/13 04:07, Vinayak Kale wrote:
> > This patch series adds support to handle interrupt registration/deregistration
> > in arm64 pmu driver when pmu interrupt type is percpu.
> >
> > Patches in this patch series were previously sent out as separate patches [1].
> > This patch series incorporates comments/fixes suggested for original patches.
> >
> > [1] 
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html 
> >
> > Vinayak Kale (2):
> >   genirq: error reporting in request_percpu_irq() and
> >     request_threaded_irq()
> >   arm64: perf: add support for percpu pmu interrupt
> >
> >  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
> >  kernel/irq/manage.c            |   12 +++--
> >  2 files changed, 89 insertions(+), 32 deletions(-)
> >
> 
> What ever happened to the approach here[1]? It doesn't look very nice to
> have to request the irq first as a per-cpu interrupt and then try as a
> non-percpu interrupt when genirq already knows if its per-cpu or not.
> 
> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html

Hmm, I'd completely forgotten about that approach. Whilst it certainly looks
cleaner from a user perspective, I always get scared when I see
'desc->status_use_accessors' since it tends to incur the wrath of tglx :)

That said, I guess that should be fine in irqdesc.h (basically adding a new
accessor). Chris went missing after sending those initial patches, so
perhaps Vinayak could look at resurrecting those?

Will

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
  2013-11-11 10:44     ` Will Deacon
@ 2013-11-11 12:33       ` Ganapat Rao P Kulkarni
  -1 siblings, 0 replies; 24+ messages in thread
From: Ganapat Rao P Kulkarni @ 2013-11-11 12:33 UTC (permalink / raw)
  To: Will Deacon
  Cc: Stephen Boyd, jcm, patches, linux-kernel, tglx, Vinayak Kale,
	linux-arm-kernel

Will,

armv8pmu_read_counter() and armv8pmu_write_counter() treats cycle
counter as 32 bit counter.
AFAIK, the cycle counter  on ARM64 is 64 bit and overflow interrupt
comes either 32 bit changes or when 64 bit flips.
and HRM says, ARM deprecates use of PMCR_EL0.LC = 0   => cycle counter
of 64 bit only

My understanding is perf's common code designed based on 32 bit cycle counters.
what is the plan? are we going to maintain the same logic of 32 bit
counter with  writing 1 to upper 32 bits of 64 bit counter?

Vinayak,

I think, it will be helpful if you mention in patch description about
changes needed in dts file (like PMUIRQ number, PPI mode etc) to
migrate to PMUv3 which is PPI.



regards
Ganapat
PS: Sending again, since my previous email was not plain-text.


On Mon, Nov 11, 2013 at 4:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Sat, Nov 09, 2013 at 01:04:23AM +0000, Stephen Boyd wrote:
>> On 11/06/13 04:07, Vinayak Kale wrote:
>> > This patch series adds support to handle interrupt registration/deregistration
>> > in arm64 pmu driver when pmu interrupt type is percpu.
>> >
>> > Patches in this patch series were previously sent out as separate patches [1].
>> > This patch series incorporates comments/fixes suggested for original patches.
>> >
>> > [1]
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html
>> >
>> > Vinayak Kale (2):
>> >   genirq: error reporting in request_percpu_irq() and
>> >     request_threaded_irq()
>> >   arm64: perf: add support for percpu pmu interrupt
>> >
>> >  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
>> >  kernel/irq/manage.c            |   12 +++--
>> >  2 files changed, 89 insertions(+), 32 deletions(-)
>> >
>>
>> What ever happened to the approach here[1]? It doesn't look very nice to
>> have to request the irq first as a per-cpu interrupt and then try as a
>> non-percpu interrupt when genirq already knows if its per-cpu or not.
>>
>> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
>
> Hmm, I'd completely forgotten about that approach. Whilst it certainly looks
> cleaner from a user perspective, I always get scared when I see
> 'desc->status_use_accessors' since it tends to incur the wrath of tglx :)
>
> That said, I guess that should be fine in irqdesc.h (basically adding a new
> accessor). Chris went missing after sending those initial patches, so
> perhaps Vinayak could look at resurrecting those?
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
regards,
Ganapat

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-11-11 12:33       ` Ganapat Rao P Kulkarni
  0 siblings, 0 replies; 24+ messages in thread
From: Ganapat Rao P Kulkarni @ 2013-11-11 12:33 UTC (permalink / raw)
  To: linux-arm-kernel

Will,

armv8pmu_read_counter() and armv8pmu_write_counter() treats cycle
counter as 32 bit counter.
AFAIK, the cycle counter  on ARM64 is 64 bit and overflow interrupt
comes either 32 bit changes or when 64 bit flips.
and HRM says, ARM deprecates use of PMCR_EL0.LC = 0   => cycle counter
of 64 bit only

My understanding is perf's common code designed based on 32 bit cycle counters.
what is the plan? are we going to maintain the same logic of 32 bit
counter with  writing 1 to upper 32 bits of 64 bit counter?

Vinayak,

I think, it will be helpful if you mention in patch description about
changes needed in dts file (like PMUIRQ number, PPI mode etc) to
migrate to PMUv3 which is PPI.



regards
Ganapat
PS: Sending again, since my previous email was not plain-text.


On Mon, Nov 11, 2013 at 4:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Sat, Nov 09, 2013 at 01:04:23AM +0000, Stephen Boyd wrote:
>> On 11/06/13 04:07, Vinayak Kale wrote:
>> > This patch series adds support to handle interrupt registration/deregistration
>> > in arm64 pmu driver when pmu interrupt type is percpu.
>> >
>> > Patches in this patch series were previously sent out as separate patches [1].
>> > This patch series incorporates comments/fixes suggested for original patches.
>> >
>> > [1]
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html
>> >
>> > Vinayak Kale (2):
>> >   genirq: error reporting in request_percpu_irq() and
>> >     request_threaded_irq()
>> >   arm64: perf: add support for percpu pmu interrupt
>> >
>> >  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
>> >  kernel/irq/manage.c            |   12 +++--
>> >  2 files changed, 89 insertions(+), 32 deletions(-)
>> >
>>
>> What ever happened to the approach here[1]? It doesn't look very nice to
>> have to request the irq first as a per-cpu interrupt and then try as a
>> non-percpu interrupt when genirq already knows if its per-cpu or not.
>>
>> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
>
> Hmm, I'd completely forgotten about that approach. Whilst it certainly looks
> cleaner from a user perspective, I always get scared when I see
> 'desc->status_use_accessors' since it tends to incur the wrath of tglx :)
>
> That said, I guess that should be fine in irqdesc.h (basically adding a new
> accessor). Chris went missing after sending those initial patches, so
> perhaps Vinayak could look at resurrecting those?
>
> Will
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



-- 
regards,
Ganapat

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
  2013-11-11 10:44     ` Will Deacon
@ 2013-11-12  6:00       ` Vinayak Kale
  -1 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-12  6:00 UTC (permalink / raw)
  To: Will Deacon
  Cc: Stephen Boyd, linux-kernel, linux-arm-kernel, patches, tglx, jcm

On Mon, Nov 11, 2013 at 4:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Sat, Nov 09, 2013 at 01:04:23AM +0000, Stephen Boyd wrote:
>> On 11/06/13 04:07, Vinayak Kale wrote:
>> > This patch series adds support to handle interrupt registration/deregistration
>> > in arm64 pmu driver when pmu interrupt type is percpu.
>> >
>> > Patches in this patch series were previously sent out as separate patches [1].
>> > This patch series incorporates comments/fixes suggested for original patches.
>> >
>> > [1]
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html
>> >
>> > Vinayak Kale (2):
>> >   genirq: error reporting in request_percpu_irq() and
>> >     request_threaded_irq()
>> >   arm64: perf: add support for percpu pmu interrupt
>> >
>> >  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
>> >  kernel/irq/manage.c            |   12 +++--
>> >  2 files changed, 89 insertions(+), 32 deletions(-)
>> >
>>
>> What ever happened to the approach here[1]? It doesn't look very nice to
>> have to request the irq first as a per-cpu interrupt and then try as a
>> non-percpu interrupt when genirq already knows if its per-cpu or not.
>>
>> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
>
> Hmm, I'd completely forgotten about that approach. Whilst it certainly looks
> cleaner from a user perspective, I always get scared when I see
> 'desc->status_use_accessors' since it tends to incur the wrath of tglx :)
>
> That said, I guess that should be fine in irqdesc.h (basically adding a new
> accessor). Chris went missing after sending those initial patches, so
> perhaps Vinayak could look at resurrecting those?
>
Okay, in next patch revision I will use that approach.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-11-12  6:00       ` Vinayak Kale
  0 siblings, 0 replies; 24+ messages in thread
From: Vinayak Kale @ 2013-11-12  6:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 11, 2013 at 4:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Sat, Nov 09, 2013 at 01:04:23AM +0000, Stephen Boyd wrote:
>> On 11/06/13 04:07, Vinayak Kale wrote:
>> > This patch series adds support to handle interrupt registration/deregistration
>> > in arm64 pmu driver when pmu interrupt type is percpu.
>> >
>> > Patches in this patch series were previously sent out as separate patches [1].
>> > This patch series incorporates comments/fixes suggested for original patches.
>> >
>> > [1]
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/205888.html
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/204414.html
>> >
>> > Vinayak Kale (2):
>> >   genirq: error reporting in request_percpu_irq() and
>> >     request_threaded_irq()
>> >   arm64: perf: add support for percpu pmu interrupt
>> >
>> >  arch/arm64/kernel/perf_event.c |  109 +++++++++++++++++++++++++++++-----------
>> >  kernel/irq/manage.c            |   12 +++--
>> >  2 files changed, 89 insertions(+), 32 deletions(-)
>> >
>>
>> What ever happened to the approach here[1]? It doesn't look very nice to
>> have to request the irq first as a per-cpu interrupt and then try as a
>> non-percpu interrupt when genirq already knows if its per-cpu or not.
>>
>> [1] http://lkml.indiana.edu/hypermail/linux/kernel/1207.3/02955.html
>
> Hmm, I'd completely forgotten about that approach. Whilst it certainly looks
> cleaner from a user perspective, I always get scared when I see
> 'desc->status_use_accessors' since it tends to incur the wrath of tglx :)
>
> That said, I guess that should be fine in irqdesc.h (basically adding a new
> accessor). Chris went missing after sending those initial patches, so
> perhaps Vinayak could look at resurrecting those?
>
Okay, in next patch revision I will use that approach.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
  2013-11-12  6:00       ` Vinayak Kale
@ 2013-11-12  8:31         ` Thomas Gleixner
  -1 siblings, 0 replies; 24+ messages in thread
From: Thomas Gleixner @ 2013-11-12  8:31 UTC (permalink / raw)
  To: Vinayak Kale
  Cc: Will Deacon, Stephen Boyd, linux-kernel, linux-arm-kernel, patches, jcm

On Tue, 12 Nov 2013, Vinayak Kale wrote:
> On Mon, Nov 11, 2013 at 4:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> > cleaner from a user perspective, I always get scared when I see
> > 'desc->status_use_accessors' since it tends to incur the wrath of tglx :)

Nah, if it uses accessors, then it's fine.

> > That said, I guess that should be fine in irqdesc.h (basically adding a new
> > accessor). Chris went missing after sending those initial patches, so
> > perhaps Vinayak could look at resurrecting those?
> >
> Okay, in next patch revision I will use that approach.

Works for me.

Thanks,

	tglx
 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt
@ 2013-11-12  8:31         ` Thomas Gleixner
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Gleixner @ 2013-11-12  8:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 12 Nov 2013, Vinayak Kale wrote:
> On Mon, Nov 11, 2013 at 4:14 PM, Will Deacon <will.deacon@arm.com> wrote:
> > cleaner from a user perspective, I always get scared when I see
> > 'desc->status_use_accessors' since it tends to incur the wrath of tglx :)

Nah, if it uses accessors, then it's fine.

> > That said, I guess that should be fine in irqdesc.h (basically adding a new
> > accessor). Chris went missing after sending those initial patches, so
> > perhaps Vinayak could look at resurrecting those?
> >
> Okay, in next patch revision I will use that approach.

Works for me.

Thanks,

	tglx
 

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2013-11-12  8:32 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-06 12:07 [PATCH 0/2] genirq: arm64: perf: support for percpu pmu interrupt Vinayak Kale
2013-11-06 12:07 ` Vinayak Kale
2013-11-06 12:07 ` [PATCH 1/2] genirq: error reporting in request_percpu_irq() and request_threaded_irq() Vinayak Kale
2013-11-06 12:07   ` Vinayak Kale
2013-11-06 12:07 ` [PATCH 2/2] arm64: perf: add support for percpu pmu interrupt Vinayak Kale
2013-11-06 12:07   ` Vinayak Kale
2013-11-08 15:57   ` Will Deacon
2013-11-08 15:57     ` Will Deacon
2013-11-11  9:30     ` Vinayak Kale
2013-11-11  9:30       ` Vinayak Kale
2013-11-09  1:04   ` Stephen Boyd
2013-11-09  1:04     ` Stephen Boyd
2013-11-09  1:04 ` [PATCH 0/2] genirq: arm64: perf: " Stephen Boyd
2013-11-09  1:04   ` Stephen Boyd
2013-11-11  9:25   ` Vinayak Kale
2013-11-11  9:25     ` Vinayak Kale
2013-11-11 10:44   ` Will Deacon
2013-11-11 10:44     ` Will Deacon
2013-11-11 12:33     ` Ganapat Rao P Kulkarni
2013-11-11 12:33       ` Ganapat Rao P Kulkarni
2013-11-12  6:00     ` Vinayak Kale
2013-11-12  6:00       ` Vinayak Kale
2013-11-12  8:31       ` Thomas Gleixner
2013-11-12  8:31         ` Thomas Gleixner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.