Skip to content

Commit

Permalink
power: fix mapped lcore ID
Browse files Browse the repository at this point in the history
This commit fixes an issue in the power library
related to using lcores mapped to different
physical cores (--lcores option in EAL).

Previously, the power library incorrectly accessed
CPU sysfs attributes for power management, treating
lcore IDs as CPU IDs.
e.g. with --lcores '1@128', lcore_id '1' was interpreted
as CPU_id instead of '128'.

This patch corrects the cpu_id based on lcore and CPU
mappings. It also constraints power management support
for lcores mapped to multiple physical cores/threads.

When multiple lcores are mapped to the same physical core,
invoking frequency scaling APIs on any lcore will apply the
changes effectively.

Fixes: 53e54bf ("eal: new option --lcores for cpu assignment")
Cc: stable@dpdk.org

Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
  • Loading branch information
sivapt12 authored and tmonjalo committed Oct 18, 2024
1 parent e214d58 commit 5c9b07e
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 7 deletions.
21 changes: 18 additions & 3 deletions app/test/test_power_cpufreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <string.h>
#include <inttypes.h>
#include <rte_cycles.h>
#include <rte_lcore.h>

#include "test.h"

Expand Down Expand Up @@ -46,9 +47,10 @@ test_power_caps(void)

static uint32_t total_freq_num;
static uint32_t freqs[TEST_POWER_FREQS_NUM_MAX];
static uint32_t cpu_id;

static int
check_cur_freq(unsigned int lcore_id, uint32_t idx, bool turbo)
check_cur_freq(__rte_unused unsigned int lcore_id, uint32_t idx, bool turbo)
{
#define TEST_POWER_CONVERT_TO_DECIMAL 10
#define MAX_LOOP 100
Expand All @@ -62,13 +64,13 @@ check_cur_freq(unsigned int lcore_id, uint32_t idx, bool turbo)
int i;

if (snprintf(fullpath, sizeof(fullpath),
TEST_POWER_SYSFILE_CPUINFO_FREQ, lcore_id) < 0) {
TEST_POWER_SYSFILE_CPUINFO_FREQ, cpu_id) < 0) {
return 0;
}
f = fopen(fullpath, "r");
if (f == NULL) {
if (snprintf(fullpath, sizeof(fullpath),
TEST_POWER_SYSFILE_SCALING_FREQ, lcore_id) < 0) {
TEST_POWER_SYSFILE_SCALING_FREQ, cpu_id) < 0) {
return 0;
}
f = fopen(fullpath, "r");
Expand Down Expand Up @@ -497,6 +499,19 @@ test_power_cpufreq(void)
{
int ret = -1;
enum power_management_env env;
rte_cpuset_t lcore_cpus;

lcore_cpus = rte_lcore_cpuset(TEST_POWER_LCORE_ID);
if (CPU_COUNT(&lcore_cpus) != 1) {
printf("Power management doesn't support lcore %u mapping to %u CPUs\n",
TEST_POWER_LCORE_ID,
CPU_COUNT(&lcore_cpus));
return TEST_SKIPPED;
}
for (cpu_id = 0; cpu_id < CPU_SETSIZE; cpu_id++) {
if (CPU_ISSET(cpu_id, &lcore_cpus))
break;
}

/* Test initialisation of a valid lcore */
ret = rte_power_init(TEST_POWER_LCORE_ID);
Expand Down
6 changes: 5 additions & 1 deletion lib/power/power_acpi_cpufreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,11 @@ power_acpi_cpufreq_init(unsigned int lcore_id)
return -1;
}

pi->lcore_id = lcore_id;
if (power_get_lcore_mapped_cpu_id(lcore_id, &pi->lcore_id) < 0) {
POWER_LOG(ERR, "Cannot get CPU ID mapped for lcore %u", lcore_id);
return -1;
}

/* Check and set the governor */
if (power_set_governor_userspace(pi) < 0) {
POWER_LOG(ERR, "Cannot set governor of lcore %u to "
Expand Down
6 changes: 5 additions & 1 deletion lib/power/power_amd_pstate_cpufreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,11 @@ power_amd_pstate_cpufreq_init(unsigned int lcore_id)
return -1;
}

pi->lcore_id = lcore_id;
if (power_get_lcore_mapped_cpu_id(lcore_id, &pi->lcore_id) < 0) {
POWER_LOG(ERR, "Cannot get CPU ID mapped for lcore %u", lcore_id);
return -1;
}

/* Check and set the governor */
if (power_set_governor_userspace(pi) < 0) {
POWER_LOG(ERR, "Cannot set governor of lcore %u to "
Expand Down
23 changes: 23 additions & 0 deletions lib/power/power_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <rte_log.h>
#include <rte_string_fns.h>
#include <rte_lcore.h>

#include "power_common.h"

Expand Down Expand Up @@ -204,3 +205,25 @@ power_set_governor(unsigned int lcore_id, const char *new_governor,

return ret;
}

int power_get_lcore_mapped_cpu_id(uint32_t lcore_id, uint32_t *cpu_id)
{
rte_cpuset_t lcore_cpus;
uint32_t cpu;

lcore_cpus = rte_lcore_cpuset(lcore_id);
if (CPU_COUNT(&lcore_cpus) != 1) {
POWER_LOG(ERR,
"Power library does not support lcore %u mapping to %u CPUs",
lcore_id, CPU_COUNT(&lcore_cpus));
return -1;
}

for (cpu = 0; cpu < CPU_SETSIZE; cpu++) {
if (CPU_ISSET(cpu, &lcore_cpus))
break;
}
*cpu_id = cpu;

return 0;
}
1 change: 1 addition & 0 deletions lib/power/power_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ int open_core_sysfs_file(FILE **f, const char *mode, const char *format, ...)
int read_core_sysfs_u32(FILE *f, uint32_t *val);
int read_core_sysfs_s(FILE *f, char *buf, unsigned int len);
int write_core_sysfs_s(FILE *f, const char *str);
int power_get_lcore_mapped_cpu_id(uint32_t lcore_id, uint32_t *cpu_id);

#endif /* _POWER_COMMON_H_ */
6 changes: 5 additions & 1 deletion lib/power/power_cppc_cpufreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,11 @@ power_cppc_cpufreq_init(unsigned int lcore_id)
return -1;
}

pi->lcore_id = lcore_id;
if (power_get_lcore_mapped_cpu_id(lcore_id, &pi->lcore_id) < 0) {
POWER_LOG(ERR, "Cannot get CPU ID mapped for lcore %u", lcore_id);
return -1;
}

/* Check and set the governor */
if (power_set_governor_userspace(pi) < 0) {
POWER_LOG(ERR, "Cannot set governor of lcore %u to "
Expand Down
6 changes: 5 additions & 1 deletion lib/power/power_pstate_cpufreq.c
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,11 @@ power_pstate_cpufreq_init(unsigned int lcore_id)
return -1;
}

pi->lcore_id = lcore_id;
if (power_get_lcore_mapped_cpu_id(lcore_id, &pi->lcore_id) < 0) {
POWER_LOG(ERR, "Cannot get CPU ID mapped for lcore %u", lcore_id);
return -1;
}

/* Check and set the governor */
if (power_set_governor_performance(pi) < 0) {
POWER_LOG(ERR, "Cannot set governor of lcore %u to "
Expand Down

0 comments on commit 5c9b07e

Please sign in to comment.