public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 1/2] perf/x86/rapl: Move the pmu allocation out of CPU hotplug
@ 2024-10-10 14:26 kan.liang
  2024-10-10 14:26 ` [PATCH V2 2/2] perf/x86/rapl: Clean up cpumask and hotplug kan.liang
  2024-10-31 10:08 ` [tip: perf/core] perf/x86/rapl: Move the pmu allocation out of CPU hotplug tip-bot2 for Kan Liang
  0 siblings, 2 replies; 5+ messages in thread
From: kan.liang @ 2024-10-10 14:26 UTC (permalink / raw)
  To: peterz, mingo, tglx, linux-kernel
  Cc: Kan Liang, Oliver Sang, Dhananjay Ugwekar

From: Kan Liang <kan.liang@linux.intel.com>

There are extra codes in the CPU hotplug function to allocate rapl pmus.
The generic PMU hotplug support is hard to be applied.

As long as the rapl pmus can be allocated upfront for each die/socket,
the code doesn't need to be implemented in the CPU hotplug function.
Move the code to the init_rapl_pmus(), and allocate a PMU for each
possible die/socket.

Tested-by: Oliver Sang <oliver.sang@intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
 arch/x86/events/rapl.c | 44 ++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index a481a939862e..7764f739fa0a 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -602,19 +602,8 @@ static int rapl_cpu_online(unsigned int cpu)
 	struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
 	int target;
 
-	if (!pmu) {
-		pmu = kzalloc_node(sizeof(*pmu), GFP_KERNEL, cpu_to_node(cpu));
-		if (!pmu)
-			return -ENOMEM;
-
-		raw_spin_lock_init(&pmu->lock);
-		INIT_LIST_HEAD(&pmu->active_list);
-		pmu->pmu = &rapl_pmus->pmu;
-		pmu->timer_interval = ms_to_ktime(rapl_timer_ms);
-		rapl_hrtimer_init(pmu);
-
-		rapl_pmus->pmus[rapl_pmu_idx] = pmu;
-	}
+	if (!pmu)
+		return -ENOMEM;
 
 	/*
 	 * Check if there is an online cpu in the package which collects rapl
@@ -707,6 +696,32 @@ static const struct attribute_group *rapl_attr_update[] = {
 	NULL,
 };
 
+static int __init init_rapl_pmu(void)
+{
+	struct rapl_pmu *pmu;
+	int idx;
+
+	for (idx = 0; idx < rapl_pmus->nr_rapl_pmu; idx++) {
+		pmu = kzalloc(sizeof(*pmu), GFP_KERNEL);
+		if (!pmu)
+			goto free;
+
+		raw_spin_lock_init(&pmu->lock);
+		INIT_LIST_HEAD(&pmu->active_list);
+		pmu->pmu = &rapl_pmus->pmu;
+		pmu->timer_interval = ms_to_ktime(rapl_timer_ms);
+		rapl_hrtimer_init(pmu);
+
+		rapl_pmus->pmus[idx] = pmu;
+	}
+
+	return 0;
+free:
+	for (; idx > 0; idx--)
+		kfree(rapl_pmus->pmus[idx - 1]);
+	return -ENOMEM;
+}
+
 static int __init init_rapl_pmus(void)
 {
 	int nr_rapl_pmu = topology_max_packages();
@@ -730,7 +745,8 @@ static int __init init_rapl_pmus(void)
 	rapl_pmus->pmu.read		= rapl_pmu_event_read;
 	rapl_pmus->pmu.module		= THIS_MODULE;
 	rapl_pmus->pmu.capabilities	= PERF_PMU_CAP_NO_EXCLUDE;
-	return 0;
+
+	return init_rapl_pmu();
 }
 
 static struct rapl_model model_snb = {
-- 
2.38.1


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

* [PATCH V2 2/2] perf/x86/rapl: Clean up cpumask and hotplug
  2024-10-10 14:26 [PATCH V2 1/2] perf/x86/rapl: Move the pmu allocation out of CPU hotplug kan.liang
@ 2024-10-10 14:26 ` kan.liang
  2024-10-14 16:07   ` Dhananjay Ugwekar
  2024-10-31 10:08   ` [tip: perf/core] " tip-bot2 for Kan Liang
  2024-10-31 10:08 ` [tip: perf/core] perf/x86/rapl: Move the pmu allocation out of CPU hotplug tip-bot2 for Kan Liang
  1 sibling, 2 replies; 5+ messages in thread
From: kan.liang @ 2024-10-10 14:26 UTC (permalink / raw)
  To: peterz, mingo, tglx, linux-kernel
  Cc: Kan Liang, Oliver Sang, Dhananjay Ugwekar

From: Kan Liang <kan.liang@linux.intel.com>

The rapl pmu is die scope, which is supported by the generic perf_event
subsystem now.

Set the scope for the rapl PMU and remove all the cpumask and hotplug
codes.

Tested-by: Oliver Sang <oliver.sang@intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
---
 arch/x86/events/rapl.c     | 90 +++-----------------------------------
 include/linux/cpuhotplug.h |  1 -
 2 files changed, 6 insertions(+), 85 deletions(-)

diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index 7764f739fa0a..0ae9fd5e619c 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -148,7 +148,6 @@ struct rapl_model {
  /* 1/2^hw_unit Joule */
 static int rapl_hw_unit[NR_RAPL_DOMAINS] __read_mostly;
 static struct rapl_pmus *rapl_pmus;
-static cpumask_t rapl_cpu_mask;
 static unsigned int rapl_cntr_mask;
 static u64 rapl_timer_ms;
 static struct perf_msr *rapl_msrs;
@@ -369,8 +368,6 @@ static int rapl_pmu_event_init(struct perf_event *event)
 	if (event->cpu < 0)
 		return -EINVAL;
 
-	event->event_caps |= PERF_EV_CAP_READ_ACTIVE_PKG;
-
 	if (!cfg || cfg >= NR_RAPL_DOMAINS + 1)
 		return -EINVAL;
 
@@ -389,7 +386,6 @@ static int rapl_pmu_event_init(struct perf_event *event)
 	pmu = cpu_to_rapl_pmu(event->cpu);
 	if (!pmu)
 		return -EINVAL;
-	event->cpu = pmu->cpu;
 	event->pmu_private = pmu;
 	event->hw.event_base = rapl_msrs[bit].msr;
 	event->hw.config = cfg;
@@ -403,23 +399,6 @@ static void rapl_pmu_event_read(struct perf_event *event)
 	rapl_event_update(event);
 }
 
-static ssize_t rapl_get_attr_cpumask(struct device *dev,
-				struct device_attribute *attr, char *buf)
-{
-	return cpumap_print_to_pagebuf(true, buf, &rapl_cpu_mask);
-}
-
-static DEVICE_ATTR(cpumask, S_IRUGO, rapl_get_attr_cpumask, NULL);
-
-static struct attribute *rapl_pmu_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL,
-};
-
-static struct attribute_group rapl_pmu_attr_group = {
-	.attrs = rapl_pmu_attrs,
-};
-
 RAPL_EVENT_ATTR_STR(energy-cores, rapl_cores, "event=0x01");
 RAPL_EVENT_ATTR_STR(energy-pkg  ,   rapl_pkg, "event=0x02");
 RAPL_EVENT_ATTR_STR(energy-ram  ,   rapl_ram, "event=0x03");
@@ -467,7 +446,6 @@ static struct attribute_group rapl_pmu_format_group = {
 };
 
 static const struct attribute_group *rapl_attr_groups[] = {
-	&rapl_pmu_attr_group,
 	&rapl_pmu_format_group,
 	&rapl_pmu_events_group,
 	NULL,
@@ -570,54 +548,6 @@ static struct perf_msr amd_rapl_msrs[] = {
 	[PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group,  NULL, false, 0 },
 };
 
-static int rapl_cpu_offline(unsigned int cpu)
-{
-	struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
-	int target;
-
-	/* Check if exiting cpu is used for collecting rapl events */
-	if (!cpumask_test_and_clear_cpu(cpu, &rapl_cpu_mask))
-		return 0;
-
-	pmu->cpu = -1;
-	/* Find a new cpu to collect rapl events */
-	target = cpumask_any_but(get_rapl_pmu_cpumask(cpu), cpu);
-
-	/* Migrate rapl events to the new target */
-	if (target < nr_cpu_ids) {
-		cpumask_set_cpu(target, &rapl_cpu_mask);
-		pmu->cpu = target;
-		perf_pmu_migrate_context(pmu->pmu, cpu, target);
-	}
-	return 0;
-}
-
-static int rapl_cpu_online(unsigned int cpu)
-{
-	s32 rapl_pmu_idx = get_rapl_pmu_idx(cpu);
-	if (rapl_pmu_idx < 0) {
-		pr_err("topology_logical_(package/die)_id() returned a negative value");
-		return -EINVAL;
-	}
-	struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
-	int target;
-
-	if (!pmu)
-		return -ENOMEM;
-
-	/*
-	 * Check if there is an online cpu in the package which collects rapl
-	 * events already.
-	 */
-	target = cpumask_any_and(&rapl_cpu_mask, get_rapl_pmu_cpumask(cpu));
-	if (target < nr_cpu_ids)
-		return 0;
-
-	cpumask_set_cpu(cpu, &rapl_cpu_mask);
-	pmu->cpu = cpu;
-	return 0;
-}
-
 static int rapl_check_hw_unit(struct rapl_model *rm)
 {
 	u64 msr_rapl_power_unit_bits;
@@ -725,9 +655,12 @@ static int __init init_rapl_pmu(void)
 static int __init init_rapl_pmus(void)
 {
 	int nr_rapl_pmu = topology_max_packages();
+	int rapl_pmu_scope = PERF_PMU_SCOPE_PKG;
 
-	if (!rapl_pmu_is_pkg_scope())
+	if (!rapl_pmu_is_pkg_scope()) {
 		nr_rapl_pmu *= topology_max_dies_per_package();
+		rapl_pmu_scope = PERF_PMU_SCOPE_DIE;
+	}
 
 	rapl_pmus = kzalloc(struct_size(rapl_pmus, pmus, nr_rapl_pmu), GFP_KERNEL);
 	if (!rapl_pmus)
@@ -743,6 +676,7 @@ static int __init init_rapl_pmus(void)
 	rapl_pmus->pmu.start		= rapl_pmu_event_start;
 	rapl_pmus->pmu.stop		= rapl_pmu_event_stop;
 	rapl_pmus->pmu.read		= rapl_pmu_event_read;
+	rapl_pmus->pmu.scope		= rapl_pmu_scope;
 	rapl_pmus->pmu.module		= THIS_MODULE;
 	rapl_pmus->pmu.capabilities	= PERF_PMU_CAP_NO_EXCLUDE;
 
@@ -892,24 +826,13 @@ static int __init rapl_pmu_init(void)
 	if (ret)
 		return ret;
 
-	/*
-	 * Install callbacks. Core will call them for each online cpu.
-	 */
-	ret = cpuhp_setup_state(CPUHP_AP_PERF_X86_RAPL_ONLINE,
-				"perf/x86/rapl:online",
-				rapl_cpu_online, rapl_cpu_offline);
-	if (ret)
-		goto out;
-
 	ret = perf_pmu_register(&rapl_pmus->pmu, "power", -1);
 	if (ret)
-		goto out1;
+		goto out;
 
 	rapl_advertise();
 	return 0;
 
-out1:
-	cpuhp_remove_state(CPUHP_AP_PERF_X86_RAPL_ONLINE);
 out:
 	pr_warn("Initialization failed (%d), disabled\n", ret);
 	cleanup_rapl_pmus();
@@ -919,7 +842,6 @@ module_init(rapl_pmu_init);
 
 static void __exit intel_rapl_exit(void)
 {
-	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_X86_RAPL_ONLINE);
 	perf_pmu_unregister(&rapl_pmus->pmu);
 	cleanup_rapl_pmus();
 }
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 2361ed4d2b15..37a9afffb59e 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -208,7 +208,6 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_X86_UNCORE_ONLINE,
 	CPUHP_AP_PERF_X86_AMD_UNCORE_ONLINE,
 	CPUHP_AP_PERF_X86_AMD_POWER_ONLINE,
-	CPUHP_AP_PERF_X86_RAPL_ONLINE,
 	CPUHP_AP_PERF_S390_CF_ONLINE,
 	CPUHP_AP_PERF_S390_SF_ONLINE,
 	CPUHP_AP_PERF_ARM_CCI_ONLINE,
-- 
2.38.1


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

* Re: [PATCH V2 2/2] perf/x86/rapl: Clean up cpumask and hotplug
  2024-10-10 14:26 ` [PATCH V2 2/2] perf/x86/rapl: Clean up cpumask and hotplug kan.liang
@ 2024-10-14 16:07   ` Dhananjay Ugwekar
  2024-10-31 10:08   ` [tip: perf/core] " tip-bot2 for Kan Liang
  1 sibling, 0 replies; 5+ messages in thread
From: Dhananjay Ugwekar @ 2024-10-14 16:07 UTC (permalink / raw)
  To: kan.liang, peterz, mingo, tglx, linux-kernel; +Cc: Oliver Sang

Hello Kan,

On 10/10/2024 7:56 PM, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> The rapl pmu is die scope, which is supported by the generic perf_event
> subsystem now.
> 
> Set the scope for the rapl PMU and remove all the cpumask and hotplug
> codes.
> 
> Tested-by: Oliver Sang <oliver.sang@intel.com>

I have tested this on Zen3 and Zen4 EPYC servers,

# perf stat -e power/energy-pkg/ sleep 1

 Performance counter stats for 'system wide':

             91.97 Joules power/energy-pkg/

       1.003791633 seconds time elapsed

Looks good to me.

Please feel free to add,

Tested-by: Dhananjay Ugwekar <dhananjay.ugwekar@amd.com>

Thanks,
Dhananjay

> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> Cc: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
> ---
>  arch/x86/events/rapl.c     | 90 +++-----------------------------------
>  include/linux/cpuhotplug.h |  1 -
>  2 files changed, 6 insertions(+), 85 deletions(-)
> 
> diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
> index 7764f739fa0a..0ae9fd5e619c 100644
> --- a/arch/x86/events/rapl.c
> +++ b/arch/x86/events/rapl.c
> @@ -148,7 +148,6 @@ struct rapl_model {
>   /* 1/2^hw_unit Joule */
>  static int rapl_hw_unit[NR_RAPL_DOMAINS] __read_mostly;
>  static struct rapl_pmus *rapl_pmus;
> -static cpumask_t rapl_cpu_mask;
>  static unsigned int rapl_cntr_mask;
>  static u64 rapl_timer_ms;
>  static struct perf_msr *rapl_msrs;
> @@ -369,8 +368,6 @@ static int rapl_pmu_event_init(struct perf_event *event)
>  	if (event->cpu < 0)
>  		return -EINVAL;
>  
> -	event->event_caps |= PERF_EV_CAP_READ_ACTIVE_PKG;
> -
>  	if (!cfg || cfg >= NR_RAPL_DOMAINS + 1)
>  		return -EINVAL;
>  
> @@ -389,7 +386,6 @@ static int rapl_pmu_event_init(struct perf_event *event)
>  	pmu = cpu_to_rapl_pmu(event->cpu);
>  	if (!pmu)
>  		return -EINVAL;
> -	event->cpu = pmu->cpu;
>  	event->pmu_private = pmu;
>  	event->hw.event_base = rapl_msrs[bit].msr;
>  	event->hw.config = cfg;
> @@ -403,23 +399,6 @@ static void rapl_pmu_event_read(struct perf_event *event)
>  	rapl_event_update(event);
>  }
>  
> -static ssize_t rapl_get_attr_cpumask(struct device *dev,
> -				struct device_attribute *attr, char *buf)
> -{
> -	return cpumap_print_to_pagebuf(true, buf, &rapl_cpu_mask);
> -}
> -
> -static DEVICE_ATTR(cpumask, S_IRUGO, rapl_get_attr_cpumask, NULL);
> -
> -static struct attribute *rapl_pmu_attrs[] = {
> -	&dev_attr_cpumask.attr,
> -	NULL,
> -};
> -
> -static struct attribute_group rapl_pmu_attr_group = {
> -	.attrs = rapl_pmu_attrs,
> -};
> -
>  RAPL_EVENT_ATTR_STR(energy-cores, rapl_cores, "event=0x01");
>  RAPL_EVENT_ATTR_STR(energy-pkg  ,   rapl_pkg, "event=0x02");
>  RAPL_EVENT_ATTR_STR(energy-ram  ,   rapl_ram, "event=0x03");
> @@ -467,7 +446,6 @@ static struct attribute_group rapl_pmu_format_group = {
>  };
>  
>  static const struct attribute_group *rapl_attr_groups[] = {
> -	&rapl_pmu_attr_group,
>  	&rapl_pmu_format_group,
>  	&rapl_pmu_events_group,
>  	NULL,
> @@ -570,54 +548,6 @@ static struct perf_msr amd_rapl_msrs[] = {
>  	[PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group,  NULL, false, 0 },
>  };
>  
> -static int rapl_cpu_offline(unsigned int cpu)
> -{
> -	struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
> -	int target;
> -
> -	/* Check if exiting cpu is used for collecting rapl events */
> -	if (!cpumask_test_and_clear_cpu(cpu, &rapl_cpu_mask))
> -		return 0;
> -
> -	pmu->cpu = -1;
> -	/* Find a new cpu to collect rapl events */
> -	target = cpumask_any_but(get_rapl_pmu_cpumask(cpu), cpu);
> -
> -	/* Migrate rapl events to the new target */
> -	if (target < nr_cpu_ids) {
> -		cpumask_set_cpu(target, &rapl_cpu_mask);
> -		pmu->cpu = target;
> -		perf_pmu_migrate_context(pmu->pmu, cpu, target);
> -	}
> -	return 0;
> -}
> -
> -static int rapl_cpu_online(unsigned int cpu)
> -{
> -	s32 rapl_pmu_idx = get_rapl_pmu_idx(cpu);
> -	if (rapl_pmu_idx < 0) {
> -		pr_err("topology_logical_(package/die)_id() returned a negative value");
> -		return -EINVAL;
> -	}
> -	struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
> -	int target;
> -
> -	if (!pmu)
> -		return -ENOMEM;
> -
> -	/*
> -	 * Check if there is an online cpu in the package which collects rapl
> -	 * events already.
> -	 */
> -	target = cpumask_any_and(&rapl_cpu_mask, get_rapl_pmu_cpumask(cpu));
> -	if (target < nr_cpu_ids)
> -		return 0;
> -
> -	cpumask_set_cpu(cpu, &rapl_cpu_mask);
> -	pmu->cpu = cpu;
> -	return 0;
> -}
> -
>  static int rapl_check_hw_unit(struct rapl_model *rm)
>  {
>  	u64 msr_rapl_power_unit_bits;
> @@ -725,9 +655,12 @@ static int __init init_rapl_pmu(void)
>  static int __init init_rapl_pmus(void)
>  {
>  	int nr_rapl_pmu = topology_max_packages();
> +	int rapl_pmu_scope = PERF_PMU_SCOPE_PKG;
>  
> -	if (!rapl_pmu_is_pkg_scope())
> +	if (!rapl_pmu_is_pkg_scope()) {
>  		nr_rapl_pmu *= topology_max_dies_per_package();
> +		rapl_pmu_scope = PERF_PMU_SCOPE_DIE;
> +	}
>  
>  	rapl_pmus = kzalloc(struct_size(rapl_pmus, pmus, nr_rapl_pmu), GFP_KERNEL);
>  	if (!rapl_pmus)
> @@ -743,6 +676,7 @@ static int __init init_rapl_pmus(void)
>  	rapl_pmus->pmu.start		= rapl_pmu_event_start;
>  	rapl_pmus->pmu.stop		= rapl_pmu_event_stop;
>  	rapl_pmus->pmu.read		= rapl_pmu_event_read;
> +	rapl_pmus->pmu.scope		= rapl_pmu_scope;
>  	rapl_pmus->pmu.module		= THIS_MODULE;
>  	rapl_pmus->pmu.capabilities	= PERF_PMU_CAP_NO_EXCLUDE;
>  
> @@ -892,24 +826,13 @@ static int __init rapl_pmu_init(void)
>  	if (ret)
>  		return ret;
>  
> -	/*
> -	 * Install callbacks. Core will call them for each online cpu.
> -	 */
> -	ret = cpuhp_setup_state(CPUHP_AP_PERF_X86_RAPL_ONLINE,
> -				"perf/x86/rapl:online",
> -				rapl_cpu_online, rapl_cpu_offline);
> -	if (ret)
> -		goto out;
> -
>  	ret = perf_pmu_register(&rapl_pmus->pmu, "power", -1);
>  	if (ret)
> -		goto out1;
> +		goto out;
>  
>  	rapl_advertise();
>  	return 0;
>  
> -out1:
> -	cpuhp_remove_state(CPUHP_AP_PERF_X86_RAPL_ONLINE);
>  out:
>  	pr_warn("Initialization failed (%d), disabled\n", ret);
>  	cleanup_rapl_pmus();
> @@ -919,7 +842,6 @@ module_init(rapl_pmu_init);
>  
>  static void __exit intel_rapl_exit(void)
>  {
> -	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_X86_RAPL_ONLINE);
>  	perf_pmu_unregister(&rapl_pmus->pmu);
>  	cleanup_rapl_pmus();
>  }
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 2361ed4d2b15..37a9afffb59e 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -208,7 +208,6 @@ enum cpuhp_state {
>  	CPUHP_AP_PERF_X86_UNCORE_ONLINE,
>  	CPUHP_AP_PERF_X86_AMD_UNCORE_ONLINE,
>  	CPUHP_AP_PERF_X86_AMD_POWER_ONLINE,
> -	CPUHP_AP_PERF_X86_RAPL_ONLINE,
>  	CPUHP_AP_PERF_S390_CF_ONLINE,
>  	CPUHP_AP_PERF_S390_SF_ONLINE,
>  	CPUHP_AP_PERF_ARM_CCI_ONLINE,

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

* [tip: perf/core] perf/x86/rapl: Clean up cpumask and hotplug
  2024-10-10 14:26 ` [PATCH V2 2/2] perf/x86/rapl: Clean up cpumask and hotplug kan.liang
  2024-10-14 16:07   ` Dhananjay Ugwekar
@ 2024-10-31 10:08   ` tip-bot2 for Kan Liang
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for Kan Liang @ 2024-10-31 10:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Kan Liang, Peter Zijlstra (Intel), Oliver Sang, Dhananjay Ugwekar,
	x86, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     9e9af8bbb5f9b565b9faf691f96f661791e199b0
Gitweb:        https://git.kernel.org/tip/9e9af8bbb5f9b565b9faf691f96f661791e199b0
Author:        Kan Liang <kan.liang@linux.intel.com>
AuthorDate:    Thu, 10 Oct 2024 07:26:04 -07:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 30 Oct 2024 22:42:19 +01:00

perf/x86/rapl: Clean up cpumask and hotplug

The rapl pmu is die scope, which is supported by the generic perf_event
subsystem now.

Set the scope for the rapl PMU and remove all the cpumask and hotplug
codes.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Oliver Sang <oliver.sang@intel.com>
Tested-by: Dhananjay Ugwekar <dhananjay.ugwekar@amd.com>
Link: https://lore.kernel.org/r/20241010142604.770192-2-kan.liang@linux.intel.com
---
 arch/x86/events/rapl.c     | 90 ++-----------------------------------
 include/linux/cpuhotplug.h |  1 +-
 2 files changed, 6 insertions(+), 85 deletions(-)

diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index 86cbee1..a8defc8 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -148,7 +148,6 @@ struct rapl_model {
  /* 1/2^hw_unit Joule */
 static int rapl_hw_unit[NR_RAPL_DOMAINS] __read_mostly;
 static struct rapl_pmus *rapl_pmus;
-static cpumask_t rapl_cpu_mask;
 static unsigned int rapl_cntr_mask;
 static u64 rapl_timer_ms;
 static struct perf_msr *rapl_msrs;
@@ -369,8 +368,6 @@ static int rapl_pmu_event_init(struct perf_event *event)
 	if (event->cpu < 0)
 		return -EINVAL;
 
-	event->event_caps |= PERF_EV_CAP_READ_ACTIVE_PKG;
-
 	if (!cfg || cfg >= NR_RAPL_DOMAINS + 1)
 		return -EINVAL;
 
@@ -389,7 +386,6 @@ static int rapl_pmu_event_init(struct perf_event *event)
 	pmu = cpu_to_rapl_pmu(event->cpu);
 	if (!pmu)
 		return -EINVAL;
-	event->cpu = pmu->cpu;
 	event->pmu_private = pmu;
 	event->hw.event_base = rapl_msrs[bit].msr;
 	event->hw.config = cfg;
@@ -403,23 +399,6 @@ static void rapl_pmu_event_read(struct perf_event *event)
 	rapl_event_update(event);
 }
 
-static ssize_t rapl_get_attr_cpumask(struct device *dev,
-				struct device_attribute *attr, char *buf)
-{
-	return cpumap_print_to_pagebuf(true, buf, &rapl_cpu_mask);
-}
-
-static DEVICE_ATTR(cpumask, S_IRUGO, rapl_get_attr_cpumask, NULL);
-
-static struct attribute *rapl_pmu_attrs[] = {
-	&dev_attr_cpumask.attr,
-	NULL,
-};
-
-static struct attribute_group rapl_pmu_attr_group = {
-	.attrs = rapl_pmu_attrs,
-};
-
 RAPL_EVENT_ATTR_STR(energy-cores, rapl_cores, "event=0x01");
 RAPL_EVENT_ATTR_STR(energy-pkg  ,   rapl_pkg, "event=0x02");
 RAPL_EVENT_ATTR_STR(energy-ram  ,   rapl_ram, "event=0x03");
@@ -467,7 +446,6 @@ static struct attribute_group rapl_pmu_format_group = {
 };
 
 static const struct attribute_group *rapl_attr_groups[] = {
-	&rapl_pmu_attr_group,
 	&rapl_pmu_format_group,
 	&rapl_pmu_events_group,
 	NULL,
@@ -570,54 +548,6 @@ static struct perf_msr amd_rapl_msrs[] = {
 	[PERF_RAPL_PSYS] = { 0, &rapl_events_psys_group,  NULL, false, 0 },
 };
 
-static int rapl_cpu_offline(unsigned int cpu)
-{
-	struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
-	int target;
-
-	/* Check if exiting cpu is used for collecting rapl events */
-	if (!cpumask_test_and_clear_cpu(cpu, &rapl_cpu_mask))
-		return 0;
-
-	pmu->cpu = -1;
-	/* Find a new cpu to collect rapl events */
-	target = cpumask_any_but(get_rapl_pmu_cpumask(cpu), cpu);
-
-	/* Migrate rapl events to the new target */
-	if (target < nr_cpu_ids) {
-		cpumask_set_cpu(target, &rapl_cpu_mask);
-		pmu->cpu = target;
-		perf_pmu_migrate_context(pmu->pmu, cpu, target);
-	}
-	return 0;
-}
-
-static int rapl_cpu_online(unsigned int cpu)
-{
-	s32 rapl_pmu_idx = get_rapl_pmu_idx(cpu);
-	if (rapl_pmu_idx < 0) {
-		pr_err("topology_logical_(package/die)_id() returned a negative value");
-		return -EINVAL;
-	}
-	struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
-	int target;
-
-	if (!pmu)
-		return -ENOMEM;
-
-	/*
-	 * Check if there is an online cpu in the package which collects rapl
-	 * events already.
-	 */
-	target = cpumask_any_and(&rapl_cpu_mask, get_rapl_pmu_cpumask(cpu));
-	if (target < nr_cpu_ids)
-		return 0;
-
-	cpumask_set_cpu(cpu, &rapl_cpu_mask);
-	pmu->cpu = cpu;
-	return 0;
-}
-
 static int rapl_check_hw_unit(struct rapl_model *rm)
 {
 	u64 msr_rapl_power_unit_bits;
@@ -725,9 +655,12 @@ free:
 static int __init init_rapl_pmus(void)
 {
 	int nr_rapl_pmu = topology_max_packages();
+	int rapl_pmu_scope = PERF_PMU_SCOPE_PKG;
 
-	if (!rapl_pmu_is_pkg_scope())
+	if (!rapl_pmu_is_pkg_scope()) {
 		nr_rapl_pmu *= topology_max_dies_per_package();
+		rapl_pmu_scope = PERF_PMU_SCOPE_DIE;
+	}
 
 	rapl_pmus = kzalloc(struct_size(rapl_pmus, pmus, nr_rapl_pmu), GFP_KERNEL);
 	if (!rapl_pmus)
@@ -743,6 +676,7 @@ static int __init init_rapl_pmus(void)
 	rapl_pmus->pmu.start		= rapl_pmu_event_start;
 	rapl_pmus->pmu.stop		= rapl_pmu_event_stop;
 	rapl_pmus->pmu.read		= rapl_pmu_event_read;
+	rapl_pmus->pmu.scope		= rapl_pmu_scope;
 	rapl_pmus->pmu.module		= THIS_MODULE;
 	rapl_pmus->pmu.capabilities	= PERF_PMU_CAP_NO_EXCLUDE;
 
@@ -892,24 +826,13 @@ static int __init rapl_pmu_init(void)
 	if (ret)
 		return ret;
 
-	/*
-	 * Install callbacks. Core will call them for each online cpu.
-	 */
-	ret = cpuhp_setup_state(CPUHP_AP_PERF_X86_RAPL_ONLINE,
-				"perf/x86/rapl:online",
-				rapl_cpu_online, rapl_cpu_offline);
-	if (ret)
-		goto out;
-
 	ret = perf_pmu_register(&rapl_pmus->pmu, "power", -1);
 	if (ret)
-		goto out1;
+		goto out;
 
 	rapl_advertise();
 	return 0;
 
-out1:
-	cpuhp_remove_state(CPUHP_AP_PERF_X86_RAPL_ONLINE);
 out:
 	pr_warn("Initialization failed (%d), disabled\n", ret);
 	cleanup_rapl_pmus();
@@ -919,7 +842,6 @@ module_init(rapl_pmu_init);
 
 static void __exit intel_rapl_exit(void)
 {
-	cpuhp_remove_state_nocalls(CPUHP_AP_PERF_X86_RAPL_ONLINE);
 	perf_pmu_unregister(&rapl_pmus->pmu);
 	cleanup_rapl_pmus();
 }
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 2361ed4..37a9aff 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -208,7 +208,6 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_X86_UNCORE_ONLINE,
 	CPUHP_AP_PERF_X86_AMD_UNCORE_ONLINE,
 	CPUHP_AP_PERF_X86_AMD_POWER_ONLINE,
-	CPUHP_AP_PERF_X86_RAPL_ONLINE,
 	CPUHP_AP_PERF_S390_CF_ONLINE,
 	CPUHP_AP_PERF_S390_SF_ONLINE,
 	CPUHP_AP_PERF_ARM_CCI_ONLINE,

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

* [tip: perf/core] perf/x86/rapl: Move the pmu allocation out of CPU hotplug
  2024-10-10 14:26 [PATCH V2 1/2] perf/x86/rapl: Move the pmu allocation out of CPU hotplug kan.liang
  2024-10-10 14:26 ` [PATCH V2 2/2] perf/x86/rapl: Clean up cpumask and hotplug kan.liang
@ 2024-10-31 10:08 ` tip-bot2 for Kan Liang
  1 sibling, 0 replies; 5+ messages in thread
From: tip-bot2 for Kan Liang @ 2024-10-31 10:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Kan Liang, Peter Zijlstra (Intel), Oliver Sang, x86, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     9b99d65c0bb4e37013bc2ec9c32b78c5751ff952
Gitweb:        https://git.kernel.org/tip/9b99d65c0bb4e37013bc2ec9c32b78c5751ff952
Author:        Kan Liang <kan.liang@linux.intel.com>
AuthorDate:    Thu, 10 Oct 2024 07:26:03 -07:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 30 Oct 2024 22:42:18 +01:00

perf/x86/rapl: Move the pmu allocation out of CPU hotplug

There are extra codes in the CPU hotplug function to allocate rapl pmus.
The generic PMU hotplug support is hard to be applied.

As long as the rapl pmus can be allocated upfront for each die/socket,
the code doesn't need to be implemented in the CPU hotplug function.
Move the code to the init_rapl_pmus(), and allocate a PMU for each
possible die/socket.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: Oliver Sang <oliver.sang@intel.com>
Link: https://lore.kernel.org/r/20241010142604.770192-1-kan.liang@linux.intel.com
---
 arch/x86/events/rapl.c | 44 +++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index a481a93..86cbee1 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -602,19 +602,8 @@ static int rapl_cpu_online(unsigned int cpu)
 	struct rapl_pmu *pmu = cpu_to_rapl_pmu(cpu);
 	int target;
 
-	if (!pmu) {
-		pmu = kzalloc_node(sizeof(*pmu), GFP_KERNEL, cpu_to_node(cpu));
-		if (!pmu)
-			return -ENOMEM;
-
-		raw_spin_lock_init(&pmu->lock);
-		INIT_LIST_HEAD(&pmu->active_list);
-		pmu->pmu = &rapl_pmus->pmu;
-		pmu->timer_interval = ms_to_ktime(rapl_timer_ms);
-		rapl_hrtimer_init(pmu);
-
-		rapl_pmus->pmus[rapl_pmu_idx] = pmu;
-	}
+	if (!pmu)
+		return -ENOMEM;
 
 	/*
 	 * Check if there is an online cpu in the package which collects rapl
@@ -707,6 +696,32 @@ static const struct attribute_group *rapl_attr_update[] = {
 	NULL,
 };
 
+static int __init init_rapl_pmu(void)
+{
+	struct rapl_pmu *pmu;
+	int idx;
+
+	for (idx = 0; idx < rapl_pmus->nr_rapl_pmu; idx++) {
+		pmu = kzalloc(sizeof(*pmu), GFP_KERNEL);
+		if (!pmu)
+			goto free;
+
+		raw_spin_lock_init(&pmu->lock);
+		INIT_LIST_HEAD(&pmu->active_list);
+		pmu->pmu = &rapl_pmus->pmu;
+		pmu->timer_interval = ms_to_ktime(rapl_timer_ms);
+		rapl_hrtimer_init(pmu);
+
+		rapl_pmus->pmus[idx] = pmu;
+	}
+
+	return 0;
+free:
+	for (; idx > 0; idx--)
+		kfree(rapl_pmus->pmus[idx - 1]);
+	return -ENOMEM;
+}
+
 static int __init init_rapl_pmus(void)
 {
 	int nr_rapl_pmu = topology_max_packages();
@@ -730,7 +745,8 @@ static int __init init_rapl_pmus(void)
 	rapl_pmus->pmu.read		= rapl_pmu_event_read;
 	rapl_pmus->pmu.module		= THIS_MODULE;
 	rapl_pmus->pmu.capabilities	= PERF_PMU_CAP_NO_EXCLUDE;
-	return 0;
+
+	return init_rapl_pmu();
 }
 
 static struct rapl_model model_snb = {

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

end of thread, other threads:[~2024-10-31 10:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-10 14:26 [PATCH V2 1/2] perf/x86/rapl: Move the pmu allocation out of CPU hotplug kan.liang
2024-10-10 14:26 ` [PATCH V2 2/2] perf/x86/rapl: Clean up cpumask and hotplug kan.liang
2024-10-14 16:07   ` Dhananjay Ugwekar
2024-10-31 10:08   ` [tip: perf/core] " tip-bot2 for Kan Liang
2024-10-31 10:08 ` [tip: perf/core] perf/x86/rapl: Move the pmu allocation out of CPU hotplug tip-bot2 for Kan Liang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox