Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1442651 > unrolled thread

[patch V2 35/67] arm/perf: Convert to hotplug state machine

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [patch V2 35/67] arm/perf: Convert to hotplug state machine Anna-Maria Gleixner <anna-maria@linutronix.de> - 2016-07-13 19:30 +0200
    Re: [patch V2 35/67] arm/perf: Convert to hotplug state machine Mark Rutland <mark.rutland@arm.com> - 2016-07-15 15:10 +0200
      Re: [patch V2 35/67] arm/perf: Convert to hotplug state machine Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-07-15 17:30 +0200
        Re: [patch V2 35/67] arm/perf: Convert to hotplug state machine Mark Rutland <mark.rutland@arm.com> - 2016-07-15 18:20 +0200
    [tip:smp/hotplug] arm/perf: Convert to hotplug state machine tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2016-07-19 09:20 +0200
      Re: [tip:smp/hotplug] arm/perf: Convert to hotplug state machine Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-07-19 09:40 +0200
        Re: [tip:smp/hotplug] arm/perf: Convert to hotplug state machine Ingo Molnar <mingo@kernel.org> - 2016-07-19 09:50 +0200
          [PATCH v2] arm/perf: Convert to hotplug state machine Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-07-19 11:10 +0200
            Re: [PATCH v2] arm/perf: Convert to hotplug state machine Mark Rutland <mark.rutland@arm.com> - 2016-07-19 12:00 +0200
              [PATCH v3] arm/perf: Convert to hotplug state machine Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-07-19 13:20 +0200
                Re: [PATCH v3] arm/perf: Convert to hotplug state machine Ingo Molnar <mingo@kernel.org> - 2016-07-20 10:10 +0200
                [tip:smp/hotplug] arm/perf: Fix hotplug state machine conversion tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2016-07-20 12:50 +0200

#1442651 — [patch V2 35/67] arm/perf: Convert to hotplug state machine

From: Thomas Gleixner <tglx@linutronix.de>

Straight forward conversion w/o bells and whistles.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
---
 drivers/perf/arm_pmu.c       | 36 +++++++++++++++---------------------
 include/linux/cpuhotplug.h   |  1 +
 include/linux/perf/arm_pmu.h |  1 -
 3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 140436a..ae9fc6c 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -691,24 +691,15 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
  * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
  * junk values out of them.
  */
-static int cpu_pmu_notify(struct notifier_block *b, unsigned long action,
-			  void *hcpu)
+static int arm_perf_starting_cpu(unsigned int cpu)
 {
-	int cpu = (unsigned long)hcpu;
-	struct arm_pmu *pmu = container_of(b, struct arm_pmu, hotplug_nb);
-
-	if ((action & ~CPU_TASKS_FROZEN) != CPU_STARTING)
-		return NOTIFY_DONE;
-
-	if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
-		return NOTIFY_DONE;
-
-	if (pmu->reset)
-		pmu->reset(pmu);
-	else
-		return NOTIFY_DONE;
-
-	return NOTIFY_OK;
+	if (!__oprofile_cpu_pmu)
+		return 0;
+	if (!cpumask_test_cpu(cpu, &__oprofile_cpu_pmu->supported_cpus))
+		return 0;
+	if (__oprofile_cpu_pmu->reset)
+		__oprofile_cpu_pmu->reset(__oprofile_cpu_pmu);
+	return 0;
 }
 
 #ifdef CONFIG_CPU_PM
@@ -819,8 +810,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	if (!cpu_hw_events)
 		return -ENOMEM;
 
-	cpu_pmu->hotplug_nb.notifier_call = cpu_pmu_notify;
-	err = register_cpu_notifier(&cpu_pmu->hotplug_nb);
+	err = cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_STARTING,
+					"AP_PERF_ARM_STARTING",
+					arm_perf_starting_cpu, NULL);
 	if (err)
 		goto out_hw_events;
 
@@ -858,7 +850,7 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	return 0;
 
 out_unregister:
-	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
+	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_ARM_STARTING);
 out_hw_events:
 	free_percpu(cpu_hw_events);
 	return err;
@@ -867,7 +859,7 @@ out_hw_events:
 static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 {
 	cpu_pm_pmu_unregister(cpu_pmu);
-	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
+	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_ARM_STARTING);
 	free_percpu(cpu_pmu->hw_events);
 }
 
@@ -1027,6 +1019,8 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 	if (ret)
 		goto out_destroy;
 
+	WARN(__oprofile_cpu_pmu, "%s(): missing PMU strucure for CPU-hotplug\n",
+	     __func__);
 	if (!__oprofile_cpu_pmu)
 		__oprofile_cpu_pmu = pmu;
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 2e227b6..365d615 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -36,6 +36,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_X86_CSTATE_STARTING,
 	CPUHP_AP_PERF_XTENSA_STARTING,
 	CPUHP_AP_ARM_VFP_STARTING,
+	CPUHP_AP_PERF_ARM_STARTING,
 	CPUHP_AP_NOTIFY_STARTING,
 	CPUHP_AP_ONLINE,
 	CPUHP_TEARDOWN_CPU,
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index d28ac05..e6ed34e 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -109,7 +109,6 @@ struct arm_pmu {
 	DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS);
 	struct platform_device	*plat_device;
 	struct pmu_hw_events	__percpu *hw_events;
-	struct notifier_block	hotplug_nb;
 	struct notifier_block	cpu_pm_nb;
 };
 
-- 
2.8.1

[toc] | [next] | [standalone]

#1444290

Hi,

On Wed, Jul 13, 2016 at 05:16:36PM +0000, Anna-Maria Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Straight forward conversion w/o bells and whistles.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> ---
>  drivers/perf/arm_pmu.c       | 36 +++++++++++++++---------------------
>  include/linux/cpuhotplug.h   |  1 +
>  include/linux/perf/arm_pmu.h |  1 -
>  3 files changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 140436a..ae9fc6c 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -691,24 +691,15 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
>   * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
>   * junk values out of them.
>   */
> -static int cpu_pmu_notify(struct notifier_block *b, unsigned long action,
> -			  void *hcpu)
> +static int arm_perf_starting_cpu(unsigned int cpu)
>  {
> -	int cpu = (unsigned long)hcpu;
> -	struct arm_pmu *pmu = container_of(b, struct arm_pmu, hotplug_nb);
> -
> -	if ((action & ~CPU_TASKS_FROZEN) != CPU_STARTING)
> -		return NOTIFY_DONE;
> -
> -	if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
> -		return NOTIFY_DONE;
> -
> -	if (pmu->reset)
> -		pmu->reset(pmu);
> -	else
> -		return NOTIFY_DONE;
> -
> -	return NOTIFY_OK;
> +	if (!__oprofile_cpu_pmu)
> +		return 0;
> +	if (!cpumask_test_cpu(cpu, &__oprofile_cpu_pmu->supported_cpus))
> +		return 0;
> +	if (__oprofile_cpu_pmu->reset)
> +		__oprofile_cpu_pmu->reset(__oprofile_cpu_pmu);
> +	return 0;
>  }

We may have multiple PMUs (e.g. two in big.LITTLE systems), and
__oprofile_cpu_pmu only contains one of these. So this conversion is not
correct.

We were relying on the notifier list implicitly containing a list of
those PMUs. It seems like we need an explicit list here.

We keep __oprofile_cpu_pmu around for legacy 32-bit users of OProfile
(on non-hetereogeneous systems), and that's all that the variable should
be used for.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1444375

* Mark Rutland | 2016-07-15 14:08:18 [+0100]:

>Hi,
Hi Mark,

>We may have multiple PMUs (e.g. two in big.LITTLE systems), and
>__oprofile_cpu_pmu only contains one of these. So this conversion is not
>correct.
>
>We were relying on the notifier list implicitly containing a list of
>those PMUs. It seems like we need an explicit list here.
>
>We keep __oprofile_cpu_pmu around for legacy 32-bit users of OProfile
>(on non-hetereogeneous systems), and that's all that the variable should
>be used for.

By the time I'e been looking there was only one node in the .dts. Now
you say it is not only possible but likely to have more than one node.

So what we need is a list which gets extended in cpu_pmu_init() instead
of using the global pointer we have now. Correct?

>Thanks,
>Mark.

Sebastian

[toc] | [prev] | [next] | [standalone]

#1444414

On Fri, Jul 15, 2016 at 05:26:30PM +0200, Sebastian Andrzej Siewior wrote:
> * Mark Rutland | 2016-07-15 14:08:18 [+0100]:
> 
> >Hi,
> Hi Mark,
> 
> >We may have multiple PMUs (e.g. two in big.LITTLE systems), and
> >__oprofile_cpu_pmu only contains one of these. So this conversion is not
> >correct.
> >
> >We were relying on the notifier list implicitly containing a list of
> >those PMUs. It seems like we need an explicit list here.
> >
> >We keep __oprofile_cpu_pmu around for legacy 32-bit users of OProfile
> >(on non-hetereogeneous systems), and that's all that the variable should
> >be used for.
> 
> By the time I'e been looking there was only one node in the .dts. Now
> you say it is not only possible but likely to have more than one node.

Yes. For example, arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts is a
big.LITTLE system with a Cortex-A7 PMU and a Cortex-A15 PMU.

> So what we need is a list which gets extended in cpu_pmu_init() instead
> of using the global pointer we have now. Correct?

Yes, that sounds right to me, with matching removal in cpu_pmu_destroy.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1446135 — [tip:smp/hotplug] arm/perf: Convert to hotplug state machine

Commit-ID:  7d88eb695a1f5f67820e02999b949d5cfa080442
Gitweb:     http://git.kernel.org/tip/7d88eb695a1f5f67820e02999b949d5cfa080442
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 13 Jul 2016 17:16:36 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 15 Jul 2016 10:40:23 +0200

arm/perf: Convert to hotplug state machine

Straight forward conversion w/o bells and whistles.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: rt@linutronix.de
Link: http://lkml.kernel.org/r/20160713153335.794097159@linutronix.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/perf/arm_pmu.c       | 36 +++++++++++++++---------------------
 include/linux/cpuhotplug.h   |  1 +
 include/linux/perf/arm_pmu.h |  1 -
 3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 140436a..ae9fc6c 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -691,24 +691,15 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
  * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
  * junk values out of them.
  */
-static int cpu_pmu_notify(struct notifier_block *b, unsigned long action,
-			  void *hcpu)
+static int arm_perf_starting_cpu(unsigned int cpu)
 {
-	int cpu = (unsigned long)hcpu;
-	struct arm_pmu *pmu = container_of(b, struct arm_pmu, hotplug_nb);
-
-	if ((action & ~CPU_TASKS_FROZEN) != CPU_STARTING)
-		return NOTIFY_DONE;
-
-	if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
-		return NOTIFY_DONE;
-
-	if (pmu->reset)
-		pmu->reset(pmu);
-	else
-		return NOTIFY_DONE;
-
-	return NOTIFY_OK;
+	if (!__oprofile_cpu_pmu)
+		return 0;
+	if (!cpumask_test_cpu(cpu, &__oprofile_cpu_pmu->supported_cpus))
+		return 0;
+	if (__oprofile_cpu_pmu->reset)
+		__oprofile_cpu_pmu->reset(__oprofile_cpu_pmu);
+	return 0;
 }
 
 #ifdef CONFIG_CPU_PM
@@ -819,8 +810,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	if (!cpu_hw_events)
 		return -ENOMEM;
 
-	cpu_pmu->hotplug_nb.notifier_call = cpu_pmu_notify;
-	err = register_cpu_notifier(&cpu_pmu->hotplug_nb);
+	err = cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_STARTING,
+					"AP_PERF_ARM_STARTING",
+					arm_perf_starting_cpu, NULL);
 	if (err)
 		goto out_hw_events;
 
@@ -858,7 +850,7 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	return 0;
 
 out_unregister:
-	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
+	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_ARM_STARTING);
 out_hw_events:
 	free_percpu(cpu_hw_events);
 	return err;
@@ -867,7 +859,7 @@ out_hw_events:
 static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 {
 	cpu_pm_pmu_unregister(cpu_pmu);
-	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
+	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_ARM_STARTING);
 	free_percpu(cpu_pmu->hw_events);
 }
 
@@ -1027,6 +1019,8 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 	if (ret)
 		goto out_destroy;
 
+	WARN(__oprofile_cpu_pmu, "%s(): missing PMU strucure for CPU-hotplug\n",
+	     __func__);
 	if (!__oprofile_cpu_pmu)
 		__oprofile_cpu_pmu = pmu;
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 4c63cb3..a5b6a65 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -35,6 +35,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_X86_CSTATE_STARTING,
 	CPUHP_AP_PERF_XTENSA_STARTING,
 	CPUHP_AP_ARM_VFP_STARTING,
+	CPUHP_AP_PERF_ARM_STARTING,
 	CPUHP_AP_NOTIFY_STARTING,
 	CPUHP_AP_ONLINE,
 	CPUHP_TEARDOWN_CPU,
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index d28ac05..e6ed34e 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -109,7 +109,6 @@ struct arm_pmu {
 	DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS);
 	struct platform_device	*plat_device;
 	struct pmu_hw_events	__percpu *hw_events;
-	struct notifier_block	hotplug_nb;
 	struct notifier_block	cpu_pm_nb;
 };
 

[toc] | [prev] | [next] | [standalone]

#1446182 — Re: [tip:smp/hotplug] arm/perf: Convert to hotplug state machine

On 2016-07-19 00:16:15 [-0700], tip-bot for Thomas Gleixner wrote:
> Commit-ID:  7d88eb695a1f5f67820e02999b949d5cfa080442
> Gitweb:     http://git.kernel.org/tip/7d88eb695a1f5f67820e02999b949d5cfa080442
> Author:     Thomas Gleixner <tglx@linutronix.de>
> AuthorDate: Wed, 13 Jul 2016 17:16:36 +0000
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Fri, 15 Jul 2016 10:40:23 +0200
> 

Ingo please drop this. I have a new version almost done (Mark asked for
it).

Sebastian

[toc] | [prev] | [next] | [standalone]

#1446207 — Re: [tip:smp/hotplug] arm/perf: Convert to hotplug state machine

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> On 2016-07-19 00:16:15 [-0700], tip-bot for Thomas Gleixner wrote:
> > Commit-ID:  7d88eb695a1f5f67820e02999b949d5cfa080442
> > Gitweb:     http://git.kernel.org/tip/7d88eb695a1f5f67820e02999b949d5cfa080442
> > Author:     Thomas Gleixner <tglx@linutronix.de>
> > AuthorDate: Wed, 13 Jul 2016 17:16:36 +0000
> > Committer:  Ingo Molnar <mingo@kernel.org>
> > CommitDate: Fri, 15 Jul 2016 10:40:23 +0200
> > 
> 
> Ingo please drop this. I have a new version almost done (Mark asked for
> it).

Please send me an updated patch ASAP.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1446263 — [PATCH v2] arm/perf: Convert to hotplug state machine

From: Thomas Gleixner <tglx@linutronix.de>

Straight forward conversion w/o bells and whistles.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: rt@linutronix.de
[bigeasy: use arm_pmu_list to handle multiple  PMUs in the system]
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/perf/arm_pmu.c       | 59 +++++++++++++++++++++++++++-----------------
 include/linux/cpuhotplug.h   |  1 +
 include/linux/perf/arm_pmu.h |  2 +-
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 140436a046c0..b7a64c0ec673 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -685,30 +685,29 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 	return 0;
 }
 
+static DEFINE_MUTEX(arm_pmu_mutex);
+static LIST_HEAD(arm_pmu_list);
+
 /*
  * PMU hardware loses all context when a CPU goes offline.
  * When a CPU is hotplugged back in, since some hardware registers are
  * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
  * junk values out of them.
  */
-static int cpu_pmu_notify(struct notifier_block *b, unsigned long action,
-			  void *hcpu)
+static int arm_perf_starting_cpu(unsigned int cpu)
 {
-	int cpu = (unsigned long)hcpu;
-	struct arm_pmu *pmu = container_of(b, struct arm_pmu, hotplug_nb);
+	struct arm_pmu *pmu;
 
-	if ((action & ~CPU_TASKS_FROZEN) != CPU_STARTING)
-		return NOTIFY_DONE;
+	mutex_lock(&arm_pmu_mutex);
+	list_for_each_entry(pmu, &arm_pmu_list, entry) {
 
-	if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
-		return NOTIFY_DONE;
-
-	if (pmu->reset)
-		pmu->reset(pmu);
-	else
-		return NOTIFY_DONE;
-
-	return NOTIFY_OK;
+		if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
+			continue;
+		if (pmu->reset)
+			pmu->reset(pmu);
+	}
+	mutex_unlock(&arm_pmu_mutex);
+	return 0;
 }
 
 #ifdef CONFIG_CPU_PM
@@ -819,10 +818,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	if (!cpu_hw_events)
 		return -ENOMEM;
 
-	cpu_pmu->hotplug_nb.notifier_call = cpu_pmu_notify;
-	err = register_cpu_notifier(&cpu_pmu->hotplug_nb);
-	if (err)
-		goto out_hw_events;
+	mutex_lock(&arm_pmu_mutex);
+	list_add_tail(&cpu_pmu->entry, &arm_pmu_list);
+	mutex_unlock(&arm_pmu_mutex);
 
 	err = cpu_pm_pmu_register(cpu_pmu);
 	if (err)
@@ -858,8 +856,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	return 0;
 
 out_unregister:
-	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
-out_hw_events:
+	mutex_lock(&arm_pmu_mutex);
+	list_del(&cpu_pmu->entry);
+	mutex_unlock(&arm_pmu_mutex);
 	free_percpu(cpu_hw_events);
 	return err;
 }
@@ -867,7 +866,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 {
 	cpu_pm_pmu_unregister(cpu_pmu);
-	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
+	mutex_lock(&arm_pmu_mutex);
+	list_del(&cpu_pmu->entry);
+	mutex_unlock(&arm_pmu_mutex);
 	free_percpu(cpu_pmu->hw_events);
 }
 
@@ -1044,3 +1045,17 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 	kfree(pmu);
 	return ret;
 }
+
+static int arm_pmu_hp_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_STARTING,
+					"AP_PERF_ARM_STARTING",
+					arm_perf_starting_cpu, NULL);
+	if (ret)
+		pr_err("CPU hotplug notifier for ARM PMU could not be "
+		       "registered: %d\n", ret);
+	return ret;
+}
+subsys_initcall(arm_pmu_hp_init);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 4c63cb30aee6..a5b6a6526af8 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -35,6 +35,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_X86_CSTATE_STARTING,
 	CPUHP_AP_PERF_XTENSA_STARTING,
 	CPUHP_AP_ARM_VFP_STARTING,
+	CPUHP_AP_PERF_ARM_STARTING,
 	CPUHP_AP_NOTIFY_STARTING,
 	CPUHP_AP_ONLINE,
 	CPUHP_TEARDOWN_CPU,
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index d28ac05c7f92..e18843809eec 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -109,7 +109,7 @@ struct arm_pmu {
 	DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS);
 	struct platform_device	*plat_device;
 	struct pmu_hw_events	__percpu *hw_events;
-	struct notifier_block	hotplug_nb;
+	struct list_head	entry;
 	struct notifier_block	cpu_pm_nb;
 };
 
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1446302 — Re: [PATCH v2] arm/perf: Convert to hotplug state machine

Hi,

On Tue, Jul 19, 2016 at 11:02:29AM +0200, Sebastian Andrzej Siewior wrote:
> +static int arm_pmu_hp_init(void)
> +{
> +	int ret;
> +
> +	ret = cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_STARTING,
> +					"AP_PERF_ARM_STARTING",
> +					arm_perf_starting_cpu, NULL);
> +	if (ret)
> +		pr_err("CPU hotplug notifier for ARM PMU could not be "
> +		       "registered: %d\n", ret);

Nit: Per Documentation/CodingStyle strings should always be on one line,
even if greater than 80 chars, so they're greppable. I assume that can
be fixed up when this is picked.

Otherwise, v2 looks correct to me. FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1446341 — [PATCH v3] arm/perf: Convert to hotplug state machine

From: Thomas Gleixner <tglx@linutronix.de>

Straight forward conversion w/o bells and whistles.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: rt@linutronix.de
[bigeasy@l.de: use arm_pmu_list to handle multiple PMUs in the system]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
v2…v3: >80 chars line for per_err, added Mark's Acked-by
v1…v2: use arm_pmu_list

 drivers/perf/arm_pmu.c       | 59 +++++++++++++++++++++++++++-----------------
 include/linux/cpuhotplug.h   |  1 +
 include/linux/perf/arm_pmu.h |  2 +-
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 140436a046c0..f6ab4f7f75bf 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -685,30 +685,29 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 	return 0;
 }
 
+static DEFINE_MUTEX(arm_pmu_mutex);
+static LIST_HEAD(arm_pmu_list);
+
 /*
  * PMU hardware loses all context when a CPU goes offline.
  * When a CPU is hotplugged back in, since some hardware registers are
  * UNKNOWN at reset, the PMU must be explicitly reset to avoid reading
  * junk values out of them.
  */
-static int cpu_pmu_notify(struct notifier_block *b, unsigned long action,
-			  void *hcpu)
+static int arm_perf_starting_cpu(unsigned int cpu)
 {
-	int cpu = (unsigned long)hcpu;
-	struct arm_pmu *pmu = container_of(b, struct arm_pmu, hotplug_nb);
+	struct arm_pmu *pmu;
 
-	if ((action & ~CPU_TASKS_FROZEN) != CPU_STARTING)
-		return NOTIFY_DONE;
+	mutex_lock(&arm_pmu_mutex);
+	list_for_each_entry(pmu, &arm_pmu_list, entry) {
 
-	if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
-		return NOTIFY_DONE;
-
-	if (pmu->reset)
-		pmu->reset(pmu);
-	else
-		return NOTIFY_DONE;
-
-	return NOTIFY_OK;
+		if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
+			continue;
+		if (pmu->reset)
+			pmu->reset(pmu);
+	}
+	mutex_unlock(&arm_pmu_mutex);
+	return 0;
 }
 
 #ifdef CONFIG_CPU_PM
@@ -819,10 +818,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	if (!cpu_hw_events)
 		return -ENOMEM;
 
-	cpu_pmu->hotplug_nb.notifier_call = cpu_pmu_notify;
-	err = register_cpu_notifier(&cpu_pmu->hotplug_nb);
-	if (err)
-		goto out_hw_events;
+	mutex_lock(&arm_pmu_mutex);
+	list_add_tail(&cpu_pmu->entry, &arm_pmu_list);
+	mutex_unlock(&arm_pmu_mutex);
 
 	err = cpu_pm_pmu_register(cpu_pmu);
 	if (err)
@@ -858,8 +856,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	return 0;
 
 out_unregister:
-	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
-out_hw_events:
+	mutex_lock(&arm_pmu_mutex);
+	list_del(&cpu_pmu->entry);
+	mutex_unlock(&arm_pmu_mutex);
 	free_percpu(cpu_hw_events);
 	return err;
 }
@@ -867,7 +866,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 {
 	cpu_pm_pmu_unregister(cpu_pmu);
-	unregister_cpu_notifier(&cpu_pmu->hotplug_nb);
+	mutex_lock(&arm_pmu_mutex);
+	list_del(&cpu_pmu->entry);
+	mutex_unlock(&arm_pmu_mutex);
 	free_percpu(cpu_pmu->hw_events);
 }
 
@@ -1044,3 +1045,17 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 	kfree(pmu);
 	return ret;
 }
+
+static int arm_pmu_hp_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_STARTING,
+					"AP_PERF_ARM_STARTING",
+					arm_perf_starting_cpu, NULL);
+	if (ret)
+		pr_err("CPU hotplug notifier for ARM PMU could not be registered: %d\n",
+		       ret);
+	return ret;
+}
+subsys_initcall(arm_pmu_hp_init);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 4c63cb30aee6..a5b6a6526af8 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -35,6 +35,7 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_X86_CSTATE_STARTING,
 	CPUHP_AP_PERF_XTENSA_STARTING,
 	CPUHP_AP_ARM_VFP_STARTING,
+	CPUHP_AP_PERF_ARM_STARTING,
 	CPUHP_AP_NOTIFY_STARTING,
 	CPUHP_AP_ONLINE,
 	CPUHP_TEARDOWN_CPU,
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index d28ac05c7f92..e18843809eec 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -109,7 +109,7 @@ struct arm_pmu {
 	DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS);
 	struct platform_device	*plat_device;
 	struct pmu_hw_events	__percpu *hw_events;
-	struct notifier_block	hotplug_nb;
+	struct list_head	entry;
 	struct notifier_block	cpu_pm_nb;
 };
 
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1447070 — Re: [PATCH v3] arm/perf: Convert to hotplug state machine

* Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> From: Thomas Gleixner <tglx@linutronix.de>
> 
> Straight forward conversion w/o bells and whistles.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: rt@linutronix.de
> [bigeasy@l.de: use arm_pmu_list to handle multiple PMUs in the system]
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
> v2…v3: >80 chars line for per_err, added Mark's Acked-by
> v1…v2: use arm_pmu_list

Thanks guys. I converted this commit into a fix patch with a proper changelog and 
added Sebastian's SOB and Mark's Acked-by (there's no changes to the resulting 
code) - this will better document the reasoning behind the introduction of 
'arm_pmu_list' and also avoids having to rebase 30+ commits.

The tip-bot will post the patch to this thread soon, please double check the 
result and let me know if anything is amiss!

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1447152 — [tip:smp/hotplug] arm/perf: Fix hotplug state machine conversion

Commit-ID:  37b502f121adab26ccc2769c3063f0e1272be7de
Gitweb:     http://git.kernel.org/tip/37b502f121adab26ccc2769c3063f0e1272be7de
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Wed, 20 Jul 2016 09:51:11 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 20 Jul 2016 09:57:34 +0200

arm/perf: Fix hotplug state machine conversion

Mark Rutland pointed out that this commit is incomplete:

  7d88eb695a1f ("arm/perf: Convert to hotplug state machine")

The problem is that:

 > We may have multiple PMUs (e.g. two in big.LITTLE systems), and
 > __oprofile_cpu_pmu only contains one of these. So this conversion is not
 > correct.
 >
 > We were relying on the notifier list implicitly containing a list of
 > those PMUs. It seems like we need an explicit list here.
 >
 > We keep __oprofile_cpu_pmu around for legacy 32-bit users of OProfile
 > (on non-hetereogeneous systems), and that's all that the variable should
 > be used for.

Introduce arm_pmu_list to correctly handle multiple PMUs in the system.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Anna-Maria Gleixner <anna-maria@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-tip-commits@vger.kernel.org
Cc: rt@linutronix.de
Link: http://lkml.kernel.org/r/20160719111733.GA22911@linutronix.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/perf/arm_pmu.c       | 53 +++++++++++++++++++++++++++++++-------------
 include/linux/perf/arm_pmu.h |  1 +
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index ae9fc6c..f6ab4f7 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -685,6 +685,9 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 	return 0;
 }
 
+static DEFINE_MUTEX(arm_pmu_mutex);
+static LIST_HEAD(arm_pmu_list);
+
 /*
  * PMU hardware loses all context when a CPU goes offline.
  * When a CPU is hotplugged back in, since some hardware registers are
@@ -693,12 +696,17 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
  */
 static int arm_perf_starting_cpu(unsigned int cpu)
 {
-	if (!__oprofile_cpu_pmu)
-		return 0;
-	if (!cpumask_test_cpu(cpu, &__oprofile_cpu_pmu->supported_cpus))
-		return 0;
-	if (__oprofile_cpu_pmu->reset)
-		__oprofile_cpu_pmu->reset(__oprofile_cpu_pmu);
+	struct arm_pmu *pmu;
+
+	mutex_lock(&arm_pmu_mutex);
+	list_for_each_entry(pmu, &arm_pmu_list, entry) {
+
+		if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
+			continue;
+		if (pmu->reset)
+			pmu->reset(pmu);
+	}
+	mutex_unlock(&arm_pmu_mutex);
 	return 0;
 }
 
@@ -810,11 +818,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	if (!cpu_hw_events)
 		return -ENOMEM;
 
-	err = cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_STARTING,
-					"AP_PERF_ARM_STARTING",
-					arm_perf_starting_cpu, NULL);
-	if (err)
-		goto out_hw_events;
+	mutex_lock(&arm_pmu_mutex);
+	list_add_tail(&cpu_pmu->entry, &arm_pmu_list);
+	mutex_unlock(&arm_pmu_mutex);
 
 	err = cpu_pm_pmu_register(cpu_pmu);
 	if (err)
@@ -850,8 +856,9 @@ static int cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	return 0;
 
 out_unregister:
-	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_ARM_STARTING);
-out_hw_events:
+	mutex_lock(&arm_pmu_mutex);
+	list_del(&cpu_pmu->entry);
+	mutex_unlock(&arm_pmu_mutex);
 	free_percpu(cpu_hw_events);
 	return err;
 }
@@ -859,7 +866,9 @@ out_hw_events:
 static void cpu_pmu_destroy(struct arm_pmu *cpu_pmu)
 {
 	cpu_pm_pmu_unregister(cpu_pmu);
-	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_ARM_STARTING);
+	mutex_lock(&arm_pmu_mutex);
+	list_del(&cpu_pmu->entry);
+	mutex_unlock(&arm_pmu_mutex);
 	free_percpu(cpu_pmu->hw_events);
 }
 
@@ -1019,8 +1028,6 @@ int arm_pmu_device_probe(struct platform_device *pdev,
 	if (ret)
 		goto out_destroy;
 
-	WARN(__oprofile_cpu_pmu, "%s(): missing PMU strucure for CPU-hotplug\n",
-	     __func__);
 	if (!__oprofile_cpu_pmu)
 		__oprofile_cpu_pmu = pmu;
 
@@ -1038,3 +1045,17 @@ out_free:
 	kfree(pmu);
 	return ret;
 }
+
+static int arm_pmu_hp_init(void)
+{
+	int ret;
+
+	ret = cpuhp_setup_state_nocalls(CPUHP_AP_PERF_ARM_STARTING,
+					"AP_PERF_ARM_STARTING",
+					arm_perf_starting_cpu, NULL);
+	if (ret)
+		pr_err("CPU hotplug notifier for ARM PMU could not be registered: %d\n",
+		       ret);
+	return ret;
+}
+subsys_initcall(arm_pmu_hp_init);
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index e6ed34e..e188438 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -109,6 +109,7 @@ struct arm_pmu {
 	DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS);
 	struct platform_device	*plat_device;
 	struct pmu_hw_events	__percpu *hw_events;
+	struct list_head	entry;
 	struct notifier_block	cpu_pm_nb;
 };
 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web