Re: [PATCH] powercap: intel_rapl: Expose package CPUs in associated_cpus

[Kuppuswamy Sathyanarayanan]

On 11/6/2026 10:30 AM, Dennis Chen wrote:
> Commit 525e0064f3d8 ("powercap: intel_rapl: Expose all package CPUs in
> PMU cpumask") expanded cpumask to include all CPUs in the package.
> Since perf opens an event for every CPU in cpumask, the value returned
> for power/energy-pkg/ is roughly scaled by the number of CPUs.

Thanks for catching this. You are right. perf treats cpumask as the set
of CPUs to open events on, so a package-scoped counter gets summed once
per CPU. My change broke that expectation.

> 
> On a single-socket 72-core DMR machine the reported value is roughly
> 72x higher than when perf is restricted to reading from a single CPU:
> 
> $ perf stat -e power/energy-pkg/ -- sleep 1
>  Performance counter stats for 'system wide':
> 
>           8,052.48 Joules power/energy-pkg/
> 
>        1.010483558 seconds time elapsed
> 
> $ perf stat -C 5 -e power/energy-pkg/ -- sleep 1
> 
>  Performance counter stats for 'CPU(s) 5':
> 
>             112.15 Joules power/energy-pkg/
> 
>        1.009685587 seconds time elapsed
> 
> Revert cpumask to only include the CPU with the lowest ID in the
> package and instead expose CPUs which can read from the PMU in
> associated_cpus, akin to uncore PMU drivers with per-package counters.
> 
> This retains flexibility for userspace tools while maintaining semantic
> consistency for perf's use of cpumask.
> 

Is there a user of associated_cpus? Neither perf nor turbostat reads it
today. The "any CPU in the package" support that turbostat needed comes
from commit 7537bae8b6eb ("powercap: intel_rapl: Remove incorrect CPU
check in PMU context") and from is_rp_pmu_cpu(). Your patch keeps both.

If there is no consumer, I would prefer a plain revert of 525e0064f3d8.
It fixes the issue with a smaller diff and does not add new sysfs ABI.
If you do want to keep associated_cpus, please add an entry under
Documentation/ABI for it.

> Assisted-by: LLM
> Fixes: 525e0064f3d8 ("powercap: intel_rapl: Expose all package CPUs in PMU cpumask")
> Signed-off-by: Dennis Chen <dechen@xxxxxxxxxx>

This has been in since v7.0, so please add Cc: stable@xxxxxxxxxxxxxxx.


> ---
>  drivers/powercap/intel_rapl_common.c | 43 ++++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c
> index 70cab5f08bc6..4197f7146afc 100644
> --- a/drivers/powercap/intel_rapl_common.c
> +++ b/drivers/powercap/intel_rapl_common.c
> @@ -1139,7 +1139,8 @@ static struct rapl_pmu rapl_pmu;
>  
>  /* PMU helpers */
>  
> -static void set_pmu_cpumask(struct rapl_package *rp, cpumask_var_t mask)
> +static void rapl_pmu_collect_cpus(struct rapl_package *rp, cpumask_var_t mask,
> +				  bool all_online)
>  {
>  	int cpu;
>  
> @@ -1151,9 +1152,15 @@ static void set_pmu_cpumask(struct rapl_package *rp, cpumask_var_t mask)
>  		return;
>  
>  	/* TPMI/MSR RAPL uses any CPU in the package for PMU */
> -	for_each_online_cpu(cpu)
> -		if (topology_physical_package_id(cpu) == rp->id)
> -			cpumask_set_cpu(cpu, mask);
> +	for_each_online_cpu(cpu) {
> +		if (topology_physical_package_id(cpu) != rp->id)
> +			continue;
> +
> +		cpumask_set_cpu(cpu, mask);
> +		/* Stop at first CPU in package if not all CPUs are requested */
> +		if (!all_online)
> +			return;
> +	}
>  }
>  
>  static bool is_rp_pmu_cpu(struct rapl_package *rp, int cpu)
> @@ -1430,8 +1437,7 @@ static struct attribute_group pmu_events_group = {
>  	.attrs = attrs_empty,
>  };
>  
> -static ssize_t cpumask_show(struct device *dev,
> -			    struct device_attribute *attr, char *buf)
> +static ssize_t rapl_pmu_cpus_show(char *buf, bool all_online)
>  {
>  	struct rapl_package *rp;
>  	cpumask_var_t cpu_mask;
> @@ -1444,23 +1450,36 @@ static ssize_t cpumask_show(struct device *dev,
>  
>  	cpumask_clear(cpu_mask);
>  
> -	/* Choose a cpu for each RAPL Package */
> -	list_for_each_entry(rp, &rapl_packages, plist) {
> -		set_pmu_cpumask(rp, cpu_mask);
> -	}
> +	/* Choose a set of CPUs for each RAPL Package */
> +	list_for_each_entry(rp, &rapl_packages, plist)
> +		rapl_pmu_collect_cpus(rp, cpu_mask, all_online);
>  	cpus_read_unlock();
>  
>  	ret = sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(cpu_mask));
> -
>  	free_cpumask_var(cpu_mask);
> -
>  	return ret;
>  }
>  
> +static ssize_t cpumask_show(struct device *dev __always_unused,
> +			    struct device_attribute *attr __always_unused,
> +			    char *buf)
> +{
> +	return rapl_pmu_cpus_show(buf, false);
> +}
> +
> +static ssize_t associated_cpus_show(struct device *dev __always_unused,
> +				    struct device_attribute *attr __always_unused,
> +				    char *buf)
> +{
> +	return rapl_pmu_cpus_show(buf, true);
> +}
> +
>  static DEVICE_ATTR_RO(cpumask);
> +static DEVICE_ATTR_RO(associated_cpus);
>  
>  static struct attribute *pmu_cpumask_attrs[] = {
>  	&dev_attr_cpumask.attr,
> +	&dev_attr_associated_cpus.attr,
>  	NULL
>  };
>  

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer