* [PATCH] power: fix number of uncore freqs
@ 2024-07-21 2:18 Stephen Hemminger
2024-07-22 15:57 ` [PATCH v2] " Stephen Hemminger
2024-07-22 20:15 ` [PATCH v3] " Stephen Hemminger
0 siblings, 2 replies; 5+ messages in thread
From: Stephen Hemminger @ 2024-07-21 2:18 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, Anatoly Burakov, David Hunt,
Sivaprasad Tummala, Tadhg Kearney
The number of uncore frequencies was defined in three places,
and two of them were too small leading to test failures.
All places should be using RTE_MAX_UNCORE_FREQS.
Bugzilla ID: 1499
Fixes: 60b8a661a957 ("power: add Intel uncore frequency control")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
app/test/test_power_intel_uncore.c | 4 +--
lib/power/power_intel_uncore.c | 55 ++++++++++++++++--------------
2 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/app/test/test_power_intel_uncore.c b/app/test/test_power_intel_uncore.c
index 80b45ce46e..049658627d 100644
--- a/app/test/test_power_intel_uncore.c
+++ b/app/test/test_power_intel_uncore.c
@@ -17,14 +17,12 @@ test_power_intel_uncore(void)
#include <rte_power_uncore.h>
#include <power_common.h>
-#define MAX_UNCORE_FREQS 32
-
#define VALID_PKG 0
#define VALID_DIE 0
#define INVALID_PKG (rte_power_uncore_get_num_pkgs() + 1)
#define INVALID_DIE (rte_power_uncore_get_num_dies(VALID_PKG) + 1)
#define VALID_INDEX 1
-#define INVALID_INDEX (MAX_UNCORE_FREQS + 1)
+#define INVALID_INDEX (RTE_MAX_UNCORE_FREQS + 1)
static int check_power_uncore_init(void)
{
diff --git a/lib/power/power_intel_uncore.c b/lib/power/power_intel_uncore.c
index 9c152e4ed2..c6b22b5ffb 100644
--- a/lib/power/power_intel_uncore.c
+++ b/lib/power/power_intel_uncore.c
@@ -11,7 +11,6 @@
#include "power_intel_uncore.h"
#include "power_common.h"
-#define MAX_UNCORE_FREQS 32
#define MAX_NUMA_DIE 8
#define BUS_FREQ 100000
#define FILTER_LENGTH 18
@@ -32,7 +31,7 @@
struct __rte_cache_aligned uncore_power_info {
unsigned int die; /* Core die id */
unsigned int pkg; /* Package id */
- uint32_t freqs[MAX_UNCORE_FREQS]; /* Frequency array */
+ uint32_t freqs[RTE_MAX_UNCORE_FREQS]; /* Frequency array */
uint32_t nb_freqs; /* Number of available freqs */
FILE *f_cur_min; /* FD of scaling_min */
FILE *f_cur_max; /* FD of scaling_max */
@@ -51,9 +50,10 @@ set_uncore_freq_internal(struct uncore_power_info *ui, uint32_t idx)
uint32_t target_uncore_freq, curr_max_freq;
int ret;
- if (idx >= MAX_UNCORE_FREQS || idx >= ui->nb_freqs) {
- POWER_LOG(DEBUG, "Invalid uncore frequency index %u, which "
- "should be less than %u", idx, ui->nb_freqs);
+ if (idx >= ui->nb_freqs) {
+ POWER_LOG(ERR,
+ "Invalid uncore frequency index %u, which should be less than %u",
+ idx, ui->nb_freqs);
return -1;
}
@@ -71,7 +71,7 @@ set_uncore_freq_internal(struct uncore_power_info *ui, uint32_t idx)
}
ret = read_core_sysfs_u32(ui->f_cur_max, &curr_max_freq);
if (ret < 0) {
- POWER_LOG(DEBUG, "Failed to read %s",
+ POWER_LOG(ERR, "Failed to read %s",
POWER_INTEL_UNCORE_SYSFILE_MAX_FREQ);
fclose(ui->f_cur_max);
return -1;
@@ -79,14 +79,15 @@ set_uncore_freq_internal(struct uncore_power_info *ui, uint32_t idx)
/* check this value first before fprintf value to f_cur_max, so value isn't overwritten */
if (fprintf(ui->f_cur_min, "%u", target_uncore_freq) < 0) {
- POWER_LOG(ERR, "Fail to write new uncore frequency for "
- "pkg %02u die %02u", ui->pkg, ui->die);
+ POWER_LOG(ERR, "Fail to write new uncore frequency for pkg %02u die %02u",
+ ui->pkg, ui->die);
return -1;
}
if (fprintf(ui->f_cur_max, "%u", target_uncore_freq) < 0) {
- POWER_LOG(ERR, "Fail to write new uncore frequency for "
- "pkg %02u die %02u", ui->pkg, ui->die);
+ POWER_LOG(ERR,
+ "Fail to write new uncore frequency for pkg %02u die %02u",
+ ui->pkg, ui->die);
return -1;
}
@@ -121,13 +122,13 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
open_core_sysfs_file(&f_base_max, "r", POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ,
ui->pkg, ui->die);
if (f_base_max == NULL) {
- POWER_LOG(DEBUG, "failed to open %s",
+ POWER_LOG(ERR, "failed to open %s",
POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ);
goto err;
}
ret = read_core_sysfs_u32(f_base_max, &base_max_freq);
if (ret < 0) {
- POWER_LOG(DEBUG, "Failed to read %s",
+ POWER_LOG(ERR, "Failed to read %s",
POWER_INTEL_UNCORE_SYSFILE_BASE_MAX_FREQ);
goto err;
}
@@ -136,14 +137,14 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
open_core_sysfs_file(&f_base_min, "r", POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ,
ui->pkg, ui->die);
if (f_base_min == NULL) {
- POWER_LOG(DEBUG, "failed to open %s",
+ POWER_LOG(ERR, "failed to open %s",
POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ);
goto err;
}
if (f_base_min != NULL) {
ret = read_core_sysfs_u32(f_base_min, &base_min_freq);
if (ret < 0) {
- POWER_LOG(DEBUG, "Failed to read %s",
+ POWER_LOG(ERR, "Failed to read %s",
POWER_INTEL_UNCORE_SYSFILE_BASE_MIN_FREQ);
goto err;
}
@@ -153,14 +154,14 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
open_core_sysfs_file(&f_min, "rw+", POWER_INTEL_UNCORE_SYSFILE_MIN_FREQ,
ui->pkg, ui->die);
if (f_min == NULL) {
- POWER_LOG(DEBUG, "failed to open %s",
+ POWER_LOG(ERR, "failed to open %s",
POWER_INTEL_UNCORE_SYSFILE_MIN_FREQ);
goto err;
}
if (f_min != NULL) {
ret = read_core_sysfs_u32(f_min, &min_freq);
if (ret < 0) {
- POWER_LOG(DEBUG, "Failed to read %s",
+ POWER_LOG(ERR, "Failed to read %s",
POWER_INTEL_UNCORE_SYSFILE_MIN_FREQ);
goto err;
}
@@ -170,14 +171,14 @@ power_init_for_setting_uncore_freq(struct uncore_power_info *ui)
open_core_sysfs_file(&f_max, "rw+", POWER_INTEL_UNCORE_SYSFILE_MAX_FREQ,
ui->pkg, ui->die);
if (f_max == NULL) {
- POWER_LOG(DEBUG, "failed to open %s",
+ POWER_LOG(ERR, "failed to open %s",
POWER_INTEL_UNCORE_SYSFILE_MAX_FREQ);
goto err;
}
if (f_max != NULL) {
ret = read_core_sysfs_u32(f_max, &max_freq);
if (ret < 0) {
- POWER_LOG(DEBUG, "Failed to read %s",
+ POWER_LOG(ERR, "Failed to read %s",
POWER_INTEL_UNCORE_SYSFILE_MAX_FREQ);
goto err;
}
@@ -221,7 +222,7 @@ power_get_available_uncore_freqs(struct uncore_power_info *ui)
uint32_t i, num_uncore_freqs = 0;
num_uncore_freqs = (ui->init_max_freq - ui->init_min_freq) / BUS_FREQ + 1;
- if (num_uncore_freqs >= MAX_UNCORE_FREQS) {
+ if (num_uncore_freqs >= RTE_MAX_UNCORE_FREQS) {
POWER_LOG(ERR, "Too many available uncore frequencies: %d",
num_uncore_freqs);
goto out;
@@ -250,7 +251,7 @@ check_pkg_die_values(unsigned int pkg, unsigned int die)
if (max_pkgs == 0)
return -1;
if (pkg >= max_pkgs) {
- POWER_LOG(DEBUG, "Package number %02u can not exceed %u",
+ POWER_LOG(ERR, "Package number %02u can not exceed %u",
pkg, max_pkgs);
return -1;
}
@@ -259,7 +260,7 @@ check_pkg_die_values(unsigned int pkg, unsigned int die)
if (max_dies == 0)
return -1;
if (die >= max_dies) {
- POWER_LOG(DEBUG, "Die number %02u can not exceed %u",
+ POWER_LOG(ERR, "Die number %02u can not exceed %u",
die, max_dies);
return -1;
}
@@ -282,15 +283,17 @@ power_intel_uncore_init(unsigned int pkg, unsigned int die)
/* Init for setting uncore die frequency */
if (power_init_for_setting_uncore_freq(ui) < 0) {
- POWER_LOG(DEBUG, "Cannot init for setting uncore frequency for "
- "pkg %02u die %02u", pkg, die);
+ POWER_LOG(ERR,
+ "Cannot init for setting uncore frequency for pkg %02u die %02u",
+ pkg, die);
return -1;
}
/* Get the available frequencies */
if (power_get_available_uncore_freqs(ui) < 0) {
- POWER_LOG(DEBUG, "Cannot get available uncore frequencies of "
- "pkg %02u die %02u", pkg, die);
+ POWER_LOG(ERR,
+ "Cannot get available uncore frequencies of pkg %02u die %02u",
+ pkg, die);
return -1;
}
@@ -451,7 +454,7 @@ power_intel_uncore_get_num_dies(unsigned int pkg)
if (max_pkgs == 0)
return 0;
if (pkg >= max_pkgs) {
- POWER_LOG(DEBUG, "Invalid package number");
+ POWER_LOG(ERR, "Invalid package number");
return 0;
}
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] power: fix number of uncore freqs
2024-07-21 2:18 [PATCH] power: fix number of uncore freqs Stephen Hemminger
@ 2024-07-22 15:57 ` Stephen Hemminger
2024-07-22 20:15 ` [PATCH v3] " Stephen Hemminger
1 sibling, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2024-07-22 15:57 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, Anatoly Burakov, David Hunt,
Sivaprasad Tummala, Tadhg Kearney
The number of uncore frequencies was defined in three places,
and two of them were too small leading to test failures.
All places should be using RTE_MAX_UNCORE_FREQS.
Bugzilla ID: 1499
Fixes: 60b8a661a957 ("power: add Intel uncore frequency control")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v2 - drop the debug log changes
app/test/test_power_intel_uncore.c | 4 +---
lib/power/power_intel_uncore.c | 5 ++---
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/app/test/test_power_intel_uncore.c b/app/test/test_power_intel_uncore.c
index 80b45ce46e..049658627d 100644
--- a/app/test/test_power_intel_uncore.c
+++ b/app/test/test_power_intel_uncore.c
@@ -17,14 +17,12 @@ test_power_intel_uncore(void)
#include <rte_power_uncore.h>
#include <power_common.h>
-#define MAX_UNCORE_FREQS 32
-
#define VALID_PKG 0
#define VALID_DIE 0
#define INVALID_PKG (rte_power_uncore_get_num_pkgs() + 1)
#define INVALID_DIE (rte_power_uncore_get_num_dies(VALID_PKG) + 1)
#define VALID_INDEX 1
-#define INVALID_INDEX (MAX_UNCORE_FREQS + 1)
+#define INVALID_INDEX (RTE_MAX_UNCORE_FREQS + 1)
static int check_power_uncore_init(void)
{
diff --git a/lib/power/power_intel_uncore.c b/lib/power/power_intel_uncore.c
index 9c152e4ed2..0090ddd374 100644
--- a/lib/power/power_intel_uncore.c
+++ b/lib/power/power_intel_uncore.c
@@ -11,7 +11,6 @@
#include "power_intel_uncore.h"
#include "power_common.h"
-#define MAX_UNCORE_FREQS 32
#define MAX_NUMA_DIE 8
#define BUS_FREQ 100000
#define FILTER_LENGTH 18
@@ -32,7 +31,7 @@
struct __rte_cache_aligned uncore_power_info {
unsigned int die; /* Core die id */
unsigned int pkg; /* Package id */
- uint32_t freqs[MAX_UNCORE_FREQS]; /* Frequency array */
+ uint32_t freqs[RTE_MAX_UNCORE_FREQS]; /* Frequency array */
uint32_t nb_freqs; /* Number of available freqs */
FILE *f_cur_min; /* FD of scaling_min */
FILE *f_cur_max; /* FD of scaling_max */
@@ -221,7 +220,7 @@ power_get_available_uncore_freqs(struct uncore_power_info *ui)
uint32_t i, num_uncore_freqs = 0;
num_uncore_freqs = (ui->init_max_freq - ui->init_min_freq) / BUS_FREQ + 1;
- if (num_uncore_freqs >= MAX_UNCORE_FREQS) {
+ if (num_uncore_freqs >= RTE_MAX_UNCORE_FREQS) {
POWER_LOG(ERR, "Too many available uncore frequencies: %d",
num_uncore_freqs);
goto out;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3] power: fix number of uncore freqs
2024-07-21 2:18 [PATCH] power: fix number of uncore freqs Stephen Hemminger
2024-07-22 15:57 ` [PATCH v2] " Stephen Hemminger
@ 2024-07-22 20:15 ` Stephen Hemminger
2024-07-23 11:40 ` Hunt, David
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2024-07-22 20:15 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, Anatoly Burakov, David Hunt,
Sivaprasad Tummala, Tadhg Kearney
The number of uncore frequencies was defined in three places,
and two of them were too small leading to test failures.
All places should be using RTE_MAX_UNCORE_FREQS.
Bugzilla ID: 1499
Fixes: 60b8a661a957 ("power: add Intel uncore frequency control")
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v3 - restore one change lost in V2
app/test/test_power_intel_uncore.c | 4 +---
lib/power/power_intel_uncore.c | 7 +++----
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/app/test/test_power_intel_uncore.c b/app/test/test_power_intel_uncore.c
index 80b45ce46e..049658627d 100644
--- a/app/test/test_power_intel_uncore.c
+++ b/app/test/test_power_intel_uncore.c
@@ -17,14 +17,12 @@ test_power_intel_uncore(void)
#include <rte_power_uncore.h>
#include <power_common.h>
-#define MAX_UNCORE_FREQS 32
-
#define VALID_PKG 0
#define VALID_DIE 0
#define INVALID_PKG (rte_power_uncore_get_num_pkgs() + 1)
#define INVALID_DIE (rte_power_uncore_get_num_dies(VALID_PKG) + 1)
#define VALID_INDEX 1
-#define INVALID_INDEX (MAX_UNCORE_FREQS + 1)
+#define INVALID_INDEX (RTE_MAX_UNCORE_FREQS + 1)
static int check_power_uncore_init(void)
{
diff --git a/lib/power/power_intel_uncore.c b/lib/power/power_intel_uncore.c
index 9c152e4ed2..4eb9c5900a 100644
--- a/lib/power/power_intel_uncore.c
+++ b/lib/power/power_intel_uncore.c
@@ -11,7 +11,6 @@
#include "power_intel_uncore.h"
#include "power_common.h"
-#define MAX_UNCORE_FREQS 32
#define MAX_NUMA_DIE 8
#define BUS_FREQ 100000
#define FILTER_LENGTH 18
@@ -32,7 +31,7 @@
struct __rte_cache_aligned uncore_power_info {
unsigned int die; /* Core die id */
unsigned int pkg; /* Package id */
- uint32_t freqs[MAX_UNCORE_FREQS]; /* Frequency array */
+ uint32_t freqs[RTE_MAX_UNCORE_FREQS]; /* Frequency array */
uint32_t nb_freqs; /* Number of available freqs */
FILE *f_cur_min; /* FD of scaling_min */
FILE *f_cur_max; /* FD of scaling_max */
@@ -51,7 +50,7 @@ set_uncore_freq_internal(struct uncore_power_info *ui, uint32_t idx)
uint32_t target_uncore_freq, curr_max_freq;
int ret;
- if (idx >= MAX_UNCORE_FREQS || idx >= ui->nb_freqs) {
+ if (idx >= RTE_MAX_UNCORE_FREQS || idx >= ui->nb_freqs) {
POWER_LOG(DEBUG, "Invalid uncore frequency index %u, which "
"should be less than %u", idx, ui->nb_freqs);
return -1;
@@ -221,7 +220,7 @@ power_get_available_uncore_freqs(struct uncore_power_info *ui)
uint32_t i, num_uncore_freqs = 0;
num_uncore_freqs = (ui->init_max_freq - ui->init_min_freq) / BUS_FREQ + 1;
- if (num_uncore_freqs >= MAX_UNCORE_FREQS) {
+ if (num_uncore_freqs >= RTE_MAX_UNCORE_FREQS) {
POWER_LOG(ERR, "Too many available uncore frequencies: %d",
num_uncore_freqs);
goto out;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] power: fix number of uncore freqs
2024-07-22 20:15 ` [PATCH v3] " Stephen Hemminger
@ 2024-07-23 11:40 ` Hunt, David
2024-07-23 13:32 ` Thomas Monjalon
0 siblings, 1 reply; 5+ messages in thread
From: Hunt, David @ 2024-07-23 11:40 UTC (permalink / raw)
To: Stephen Hemminger, dev; +Cc: Anatoly Burakov, Sivaprasad Tummala, Tadhg Kearney
[-- Attachment #1: Type: text/plain, Size: 636 bytes --]
On 22/07/2024 21:15, Stephen Hemminger wrote:
> The number of uncore frequencies was defined in three places,
> and two of them were too small leading to test failures.
> All places should be using RTE_MAX_UNCORE_FREQS.
>
> Bugzilla ID: 1499
> Fixes: 60b8a661a957 ("power: add Intel uncore frequency control")
> Signed-off-by: Stephen Hemminger<stephen@networkplumber.org>
> ---
>
> v3 - restore one change lost in V2
>
> app/test/test_power_intel_uncore.c | 4 +---
> lib/power/power_intel_uncore.c | 7 +++----
> 2 files changed, 4 insertions(+), 7 deletions(-)
>
--snip--
LGTM.
Acked-by: David Hunt <david.hunt@intel.com>
[-- Attachment #2: Type: text/html, Size: 1361 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v3] power: fix number of uncore freqs
2024-07-23 11:40 ` Hunt, David
@ 2024-07-23 13:32 ` Thomas Monjalon
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2024-07-23 13:32 UTC (permalink / raw)
To: Stephen Hemminger
Cc: dev, Anatoly Burakov, Sivaprasad Tummala, Tadhg Kearney, Hunt, David
23/07/2024 13:40, Hunt, David:
> On 22/07/2024 21:15, Stephen Hemminger wrote:
> > The number of uncore frequencies was defined in three places,
> > and two of them were too small leading to test failures.
> > All places should be using RTE_MAX_UNCORE_FREQS.
> >
> > Bugzilla ID: 1499
> > Fixes: 60b8a661a957 ("power: add Intel uncore frequency control")
> > Signed-off-by: Stephen Hemminger<stephen@networkplumber.org>
>
> Acked-by: David Hunt <david.hunt@intel.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-07-23 13:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-21 2:18 [PATCH] power: fix number of uncore freqs Stephen Hemminger
2024-07-22 15:57 ` [PATCH v2] " Stephen Hemminger
2024-07-22 20:15 ` [PATCH v3] " Stephen Hemminger
2024-07-23 11:40 ` Hunt, David
2024-07-23 13:32 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).