| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1746b1d2-cef6-4c32-b94d-ddec5b8709df@amd.com>
Date: Mon, 14 Oct 2024 21:37:56 +0530
From: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
To: kan.liang@...ux.intel.com, peterz@...radead.org, mingo@...nel.org,
tglx@...utronix.de, linux-kernel@...r.kernel.org
Cc: Oliver Sang <oliver.sang@...el.com>
Subject: Re: [PATCH V2 2/2] perf/x86/rapl: Clean up cpumask and hotplug
Hello Kan,
On 10/10/2024 7:56 PM, kan.liang@...ux.intel.com wrote:
> From: Kan Liang <kan.liang@...ux.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@...el.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@....com>
Thanks,
Dhananjay
> Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
> Cc: Dhananjay Ugwekar <Dhananjay.Ugwekar@....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,
Powered by blists - more mailing lists