From: Kan Liang <[email protected]>
RAPL becomes die-scope on Xeon Cascade Lake-AP. Perf RAPL driver needs
to support die-scope RAPL domain.
Use topology_logical_die_id() to replace topology_logical_package_id().
For previous platforms which doesn't have multi-die,
topology_logical_die_id() is identical as topology_logical_package_id().
Use topology_die_cpumask() to replace topology_core_cpumask().
For previous platforms which doesn't have multi-die,
topology_die_cpumask() is identical as topology_core_cpumask().
There is no functional change for previous platforms.
Signed-off-by: Kan Liang <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Signed-off-by: Len Brown <[email protected]>
---
arch/x86/events/intel/rapl.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/events/intel/rapl.c b/arch/x86/events/intel/rapl.c
index 94dc564146ca..e49f69c51b10 100644
--- a/arch/x86/events/intel/rapl.c
+++ b/arch/x86/events/intel/rapl.c
@@ -161,7 +161,7 @@ static u64 rapl_timer_ms;
static inline struct rapl_pmu *cpu_to_rapl_pmu(unsigned int cpu)
{
- unsigned int pkgid = topology_logical_package_id(cpu);
+ unsigned int pkgid = topology_logical_die_id(cpu);
/*
* The unsigned check also catches the '-1' return value for non
@@ -571,7 +571,7 @@ static int rapl_cpu_offline(unsigned int cpu)
pmu->cpu = -1;
/* Find a new cpu to collect rapl events */
- target = cpumask_any_but(topology_core_cpumask(cpu), cpu);
+ target = cpumask_any_but(topology_die_cpumask(cpu), cpu);
/* Migrate rapl events to the new target */
if (target < nr_cpu_ids) {
@@ -598,14 +598,14 @@ static int rapl_cpu_online(unsigned int cpu)
pmu->timer_interval = ms_to_ktime(rapl_timer_ms);
rapl_hrtimer_init(pmu);
- rapl_pmus->pmus[topology_logical_package_id(cpu)] = pmu;
+ rapl_pmus->pmus[topology_logical_die_id(cpu)] = pmu;
}
/*
* Check if there is an online cpu in the package which collects rapl
* events already.
*/
- target = cpumask_any_and(&rapl_cpu_mask, topology_core_cpumask(cpu));
+ target = cpumask_any_and(&rapl_cpu_mask, topology_die_cpumask(cpu));
if (target < nr_cpu_ids)
return 0;
@@ -675,7 +675,7 @@ static void cleanup_rapl_pmus(void)
static int __init init_rapl_pmus(void)
{
- int maxpkg = topology_max_packages();
+ int maxpkg = topology_max_packages() * topology_max_die_per_package();
size_t size;
size = sizeof(*rapl_pmus) + maxpkg * sizeof(struct rapl_pmu *);
--
2.18.0-rc0
* Len Brown <[email protected]> wrote:
>
> static inline struct rapl_pmu *cpu_to_rapl_pmu(unsigned int cpu)
> {
> - unsigned int pkgid = topology_logical_package_id(cpu);
> + unsigned int pkgid = topology_logical_die_id(cpu);
Yeah, I think Peter pointed this out before: why is the local variable
still named 'pkg' when we now have a die ID?
The other patches have such problems too: when new facilities are
introduced and used then function names and variable names need to be
harmonized. Makes for rather confusing code otherwise, which invites bugs
down the road.
There's many such instances left in this series, please review the whole
patchset for such problems.
Thanks,
Ingo
On Mon, May 6, 2019 at 7:48 AM Ingo Molnar <[email protected]> wrote:
> > - unsigned int pkgid = topology_logical_package_id(cpu);
> > + unsigned int pkgid = topology_logical_die_id(cpu);
> There's many such instances left in this series, please review the whole
> patchset for such problems.
I agree, the legacy names make the code read as if it were a bug, even
when it is correct.
I'll refresh the series with some re-names to make the end result ready clearly.
thanks!
Len Brown, Intel Open Source Technology Center
* Len Brown <[email protected]> wrote:
> On Mon, May 6, 2019 at 7:48 AM Ingo Molnar <[email protected]> wrote:
>
> > > - unsigned int pkgid = topology_logical_package_id(cpu);
> > > + unsigned int pkgid = topology_logical_die_id(cpu);
>
> > There's many such instances left in this series, please review the whole
> > patchset for such problems.
>
> I agree, the legacy names make the code read as if it were a bug, even
> when it is correct.
> I'll refresh the series with some re-names to make the end result ready clearly.
Thanks a lot!
Ingo