Re: [dpdk-dev] [PATCH v6 2/2] power: refactor pstate and acpi code
From: David Marchand <hidden>
Date: 2021-07-08 12:49:21
On Wed, Jun 23, 2021 at 2:04 PM David Hunt [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Anatoly Burakov <redacted> Currently, ACPI and PSTATE modes have lots of code duplication, confusing logic, and a bunch of other issues that can, and have, led to various bugs and resource leaks. This commit factors out the common parts of sysfs reading/writing for ACPI and PSTATE drivers. Signed-off-by: Anatoly Burakov <redacted> Signed-off-by: David Hunt <redacted> --- changes in v5 * fixed bugs raised by Richael Zhuang in review - open file rw+, etc. * removed FOPS* and FOPEN* macros, which contained control statements. * fixed some checkpatch warnings. changes in v6 * fixed check of fputs return, negative on error. --- lib/power/meson.build | 7 + lib/power/power_acpi_cpufreq.c | 192 ++++------------ lib/power/power_common.c | 146 ++++++++++++ lib/power/power_common.h | 17 ++ lib/power/power_pstate_cpufreq.c | 374 ++++++++++--------------------- 5 files changed, 335 insertions(+), 401 deletions(-)diff --git a/lib/power/meson.build b/lib/power/meson.build index c1097d32f1..74c5f3a294 100644 --- a/lib/power/meson.build +++ b/lib/power/meson.build@@ -5,6 +5,13 @@ if not is_linux build = false reason = 'only supported on Linux' endif + +# we do some snprintf magic so silence format-nonliteral +flag_nonliteral = '-Wno-format-nonliteral' +if cc.has_argument(flag_nonliteral) + cflags += flag_nonliteral +endif +
This can be removed with __rte_format_printf tag + API change below.
sources = files(
'guest_channel.c',
'power_acpi_cpufreq.c',[snip]
quoted hunk ↗ jump to hunk
diff --git a/lib/power/power_common.c b/lib/power/power_common.c index 67e3318ec7..4deb343dae 100644 --- a/lib/power/power_common.c +++ b/lib/power/power_common.c@@ -3,13 +3,20 @@ */ #include <limits.h> +#include <stdlib.h> #include <stdio.h> #include <string.h> +#include <rte_log.h> +#include <rte_string_fns.h> + #include "power_common.h" #define POWER_SYSFILE_SCALING_DRIVER \ "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_driver" +#define POWER_SYSFILE_GOVERNOR \ + "/sys/devices/system/cpu/cpu%u/cpufreq/scaling_governor" +#define POWER_CONVERT_TO_DECIMAL 10 int cpufreq_check_scaling_driver(const char *driver_name)@@ -58,3 +65,142 @@ cpufreq_check_scaling_driver(const char *driver_name) */ return 1; }
cpufreq_check_scaling_driver can use open_core_sysfs_file, right?
+
+int
+open_core_sysfs_file(const char *template, unsigned int core, const char *mode,
+ FILE **f)
+{
+ char fullpath[PATH_MAX];
+ FILE *tmpf;
+
+ /* silenced -Wformat-nonliteral here */
+ snprintf(fullpath, sizeof(fullpath), template, core);
+ tmpf = fopen(fullpath, mode);
+ *f = tmpf;
+ if (tmpf == NULL)
+ return -1;
+
+ return 0;
+}@@ -67,14 +67,15 @@ cpufreq_check_scaling_driver(const char *driver_name) } int -open_core_sysfs_file(const char *template, unsigned int core, const char *mode, - FILE **f) +open_core_sysfs_file(FILE **f, const char *mode, const char *format, ...) { char fullpath[PATH_MAX]; + va_list ap; FILE *tmpf; - /* silenced -Wformat-nonliteral here */ - snprintf(fullpath, sizeof(fullpath), template, core); + va_start(ap, format); + vsnprintf(fullpath, sizeof(fullpath), format, ap); + va_end(ap); tmpf = fopen(fullpath, mode); *f = tmpf; if (tmpf == NULL)
With declaration in .h as:
@@ -7,6 +7,8 @@ #include <inttypes.h> +#include <rte_common.h> + #define RTE_POWER_INVALID_FREQ_INDEX (~0)
@@ -21,8 +23,8 @@ int cpufreq_check_scaling_driver(const char *driver); int power_set_governor(unsigned int lcore_id, const char *new_governor, char *orig_governor, size_t orig_governor_len); -int open_core_sysfs_file(const char *template, unsigned int core, - const char *mode, FILE **f); +int open_core_sysfs_file(FILE **f, const char *mode, const char *format, ...) + __rte_format_printf(3, 4); 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);
This leaves the possibility to use any kind of formats. And to be honest, I did not manage to make gcc happy otherwise (even when passing __rte_format_printf(3, 0)). -- David Marchand