Patch Detail

HTTP 200 OK
Allow: GET, PUT, PATCH, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

{
    "id": 94676,
    "url": "https://patches.dpdk.org/api/patches/94676/?format=api",
    "web_url": "https://patches.dpdk.org/project/dpdk/patch/20210622125853.2798-2-david.hunt@intel.com/",
    "project": {
        "id": 1,
        "url": "https://patches.dpdk.org/api/projects/1/?format=api",
        "name": "DPDK",
        "link_name": "dpdk",
        "list_id": "dev.dpdk.org",
        "list_email": "dev@dpdk.org",
        "web_url": "http://core.dpdk.org",
        "scm_url": "git://dpdk.org/dpdk",
        "webscm_url": "http://git.dpdk.org/dpdk",
        "list_archive_url": "https://inbox.dpdk.org/dev",
        "list_archive_url_format": "https://inbox.dpdk.org/dev/{}",
        "commit_url_format": ""
    },
    "msgid": "<20210622125853.2798-2-david.hunt@intel.com>",
    "list_archive_url": "https://inbox.dpdk.org/dev/20210622125853.2798-2-david.hunt@intel.com",
    "date": "2021-06-22T12:58:53",
    "name": "[v5,2/2] power: refactor pstate and acpi code",
    "commit_ref": null,
    "pull_url": null,
    "state": "superseded",
    "archived": true,
    "hash": "0e3560042c111d81c742ed4ef78b42b464a9d476",
    "submitter": {
        "id": 342,
        "url": "https://patches.dpdk.org/api/people/342/?format=api",
        "name": "Hunt, David",
        "email": "david.hunt@intel.com"
    },
    "delegate": null,
    "mbox": "https://patches.dpdk.org/project/dpdk/patch/20210622125853.2798-2-david.hunt@intel.com/mbox/",
    "series": [
        {
            "id": 17439,
            "url": "https://patches.dpdk.org/api/series/17439/?format=api",
            "web_url": "https://patches.dpdk.org/project/dpdk/list/?series=17439",
            "date": "2021-06-22T12:58:52",
            "name": "[v5,1/2] power: don't use rte prefix in internal code",
            "version": 5,
            "mbox": "https://patches.dpdk.org/series/17439/mbox/"
        }
    ],
    "comments": "https://patches.dpdk.org/api/patches/94676/comments/",
    "check": "fail",
    "checks": "https://patches.dpdk.org/api/patches/94676/checks/",
    "tags": {},
    "related": [],
    "headers": {
        "Return-Path": "<dev-bounces@dpdk.org>",
        "X-Original-To": "patchwork@inbox.dpdk.org",
        "Delivered-To": "patchwork@inbox.dpdk.org",
        "Received": [
            "from mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 6A4BAA0548;\n\tTue, 22 Jun 2021 14:59:06 +0200 (CEST)",
            "from [217.70.189.124] (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id 8EC5440DF5;\n\tTue, 22 Jun 2021 14:59:02 +0200 (CEST)",
            "from mga14.intel.com (mga14.intel.com [192.55.52.115])\n by mails.dpdk.org (Postfix) with ESMTP id 5995C4003C\n for <dev@dpdk.org>; Tue, 22 Jun 2021 14:59:00 +0200 (CEST)",
            "from fmsmga001.fm.intel.com ([10.253.24.23])\n by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n 22 Jun 2021 05:58:59 -0700",
            "from silpixa00399952.ir.intel.com ([10.55.129.13])\n by fmsmga001.fm.intel.com with ESMTP; 22 Jun 2021 05:58:57 -0700"
        ],
        "IronPort-SDR": [
            "\n NhifjQDSY4fGxC6fUy0I/zoeK9NTbaE6Aj1pClqdSea8u0NbFocg0p2cevc2xIgqryVFkrPMq1\n ZnSyEyUB220Q==",
            "\n PVjb3/Ook/x6YcpbNZ+FsOk2/nPrsdfqNGwSWt6F05iEvoGkoZp2i3WzivjUo1OTykBm35cswK\n gLwF9kBtMW9Q=="
        ],
        "X-IronPort-AV": [
            "E=McAfee;i=\"6200,9189,10022\"; a=\"206866266\"",
            "E=Sophos;i=\"5.83,291,1616482800\"; d=\"scan'208\";a=\"206866266\"",
            "E=Sophos;i=\"5.83,291,1616482800\"; d=\"scan'208\";a=\"556606298\""
        ],
        "X-ExtLoop1": "1",
        "From": "David Hunt <david.hunt@intel.com>",
        "To": "dev@dpdk.org",
        "Cc": "david.hunt@intel.com,\n\tanatoly.burakov@intel.com",
        "Date": "Tue, 22 Jun 2021 13:58:53 +0100",
        "Message-Id": "<20210622125853.2798-2-david.hunt@intel.com>",
        "X-Mailer": "git-send-email 2.17.1",
        "In-Reply-To": "<20210622125853.2798-1-david.hunt@intel.com>",
        "References": "\n <a3d905e76229e2f77a164379af8ccb0dcdac0dbc.1619175790.git.anatoly.burakov@intel.com>\n <20210622125853.2798-1-david.hunt@intel.com>",
        "Subject": "[dpdk-dev] [PATCH v5 2/2] power: refactor pstate and acpi code",
        "X-BeenThere": "dev@dpdk.org",
        "X-Mailman-Version": "2.1.29",
        "Precedence": "list",
        "List-Id": "DPDK patches and discussions <dev.dpdk.org>",
        "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>",
        "List-Archive": "<http://mails.dpdk.org/archives/dev/>",
        "List-Post": "<mailto:dev@dpdk.org>",
        "List-Help": "<mailto:dev-request@dpdk.org?subject=help>",
        "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>",
        "Errors-To": "dev-bounces@dpdk.org",
        "Sender": "\"dev\" <dev-bounces@dpdk.org>"
    },
    "content": "From: Anatoly Burakov <anatoly.burakov@intel.com>\n\nCurrently, ACPI and PSTATE modes have lots of code duplication,\nconfusing logic, and a bunch of other issues that can, and have, led to\nvarious bugs and resource leaks.\n\nThis commit factors out the common parts of sysfs reading/writing for\nACPI and PSTATE drivers.\n\nSigned-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\nSigned-off-by: David Hunt <david.hunt@intel.com>\n\n---\nchanges in v2\n* fixed bugs raised by Richael Zhuang in review - open file rw+, etc.\n* removed FOPS* and FOPEN* macros, which contained control statements.\n* fixed some checkpatch warnings.\n---\n lib/power/meson.build            |   7 +\n lib/power/power_acpi_cpufreq.c   | 192 ++++------------\n lib/power/power_common.c         | 146 ++++++++++++\n lib/power/power_common.h         |  17 ++\n lib/power/power_pstate_cpufreq.c | 374 ++++++++++---------------------\n 5 files changed, 335 insertions(+), 401 deletions(-)",
    "diff": "diff --git a/lib/power/meson.build b/lib/power/meson.build\nindex c1097d32f1..74c5f3a294 100644\n--- a/lib/power/meson.build\n+++ b/lib/power/meson.build\n@@ -5,6 +5,13 @@ if not is_linux\n     build = false\n     reason = 'only supported on Linux'\n endif\n+\n+# we do some snprintf magic so silence format-nonliteral\n+flag_nonliteral = '-Wno-format-nonliteral'\n+if cc.has_argument(flag_nonliteral)\n+\tcflags += flag_nonliteral\n+endif\n+\n sources = files(\n         'guest_channel.c',\n         'power_acpi_cpufreq.c',\ndiff --git a/lib/power/power_acpi_cpufreq.c b/lib/power/power_acpi_cpufreq.c\nindex 1b8c69cc8b..9ca8d8a8f2 100644\n--- a/lib/power/power_acpi_cpufreq.c\n+++ b/lib/power/power_acpi_cpufreq.c\n@@ -19,41 +19,10 @@\n #include \"power_acpi_cpufreq.h\"\n #include \"power_common.h\"\n \n-#ifdef RTE_LIBRTE_POWER_DEBUG\n-#define POWER_DEBUG_TRACE(fmt, args...) do { \\\n-\t\tRTE_LOG(ERR, POWER, \"%s: \" fmt, __func__, ## args); \\\n-} while (0)\n-#else\n-#define POWER_DEBUG_TRACE(fmt, args...)\n-#endif\n-\n-#define FOPEN_OR_ERR_RET(f, retval) do { \\\n-\t\tif ((f) == NULL) { \\\n-\t\t\tRTE_LOG(ERR, POWER, \"File not opened\\n\"); \\\n-\t\t\treturn retval; \\\n-\t\t} \\\n-} while (0)\n-\n-#define FOPS_OR_NULL_GOTO(ret, label) do { \\\n-\t\tif ((ret) == NULL) { \\\n-\t\t\tRTE_LOG(ERR, POWER, \"fgets returns nothing\\n\"); \\\n-\t\t\tgoto label; \\\n-\t\t} \\\n-} while (0)\n-\n-#define FOPS_OR_ERR_GOTO(ret, label) do { \\\n-\t\tif ((ret) < 0) { \\\n-\t\t\tRTE_LOG(ERR, POWER, \"File operations failed\\n\"); \\\n-\t\t\tgoto label; \\\n-\t\t} \\\n-} while (0)\n-\n #define STR_SIZE     1024\n #define POWER_CONVERT_TO_DECIMAL 10\n \n #define POWER_GOVERNOR_USERSPACE \"userspace\"\n-#define POWER_SYSFILE_GOVERNOR   \\\n-\t\t\"/sys/devices/system/cpu/cpu%u/cpufreq/scaling_governor\"\n #define POWER_SYSFILE_AVAIL_FREQ \\\n \t\t\"/sys/devices/system/cpu/cpu%u/cpufreq/scaling_available_frequencies\"\n #define POWER_SYSFILE_SETSPEED   \\\n@@ -135,53 +104,18 @@ set_freq_internal(struct acpi_power_info *pi, uint32_t idx)\n static int\n power_set_governor_userspace(struct acpi_power_info *pi)\n {\n-\tFILE *f;\n-\tint ret = -1;\n-\tchar buf[BUFSIZ];\n-\tchar fullpath[PATH_MAX];\n-\tchar *s;\n-\tint val;\n-\n-\tsnprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_GOVERNOR,\n-\t\t\tpi->lcore_id);\n-\tf = fopen(fullpath, \"rw+\");\n-\tFOPEN_OR_ERR_RET(f, ret);\n-\n-\ts = fgets(buf, sizeof(buf), f);\n-\tFOPS_OR_NULL_GOTO(s, out);\n-\t/* Strip off terminating '\\n' */\n-\tstrtok(buf, \"\\n\");\n-\n-\t/* Save the original governor */\n-\trte_strscpy(pi->governor_ori, buf, sizeof(pi->governor_ori));\n-\n-\t/* Check if current governor is userspace */\n-\tif (strncmp(buf, POWER_GOVERNOR_USERSPACE,\n-\t\t\tsizeof(POWER_GOVERNOR_USERSPACE)) == 0) {\n-\t\tret = 0;\n-\t\tPOWER_DEBUG_TRACE(\"Power management governor of lcore %u is \"\n-\t\t\t\t\"already userspace\\n\", pi->lcore_id);\n-\t\tgoto out;\n-\t}\n-\n-\t/* Write 'userspace' to the governor */\n-\tval = fseek(f, 0, SEEK_SET);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\tval = fputs(POWER_GOVERNOR_USERSPACE, f);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\t/* We need to flush to see if the fputs succeeds */\n-\tval = fflush(f);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\tret = 0;\n-\tRTE_LOG(INFO, POWER, \"Power management governor of lcore %u has been \"\n-\t\t\t\"set to user space successfully\\n\", pi->lcore_id);\n-out:\n-\tfclose(f);\n+\treturn power_set_governor(pi->lcore_id, POWER_GOVERNOR_USERSPACE,\n+\t\t\tpi->governor_ori, sizeof(pi->governor_ori));\n+}\n \n-\treturn ret;\n+/**\n+ * It is to check the governor and then set the original governor back if\n+ * needed by writing the sys file.\n+ */\n+static int\n+power_set_governor_original(struct acpi_power_info *pi)\n+{\n+\treturn power_set_governor(pi->lcore_id, pi->governor_ori, NULL, 0);\n }\n \n /**\n@@ -195,22 +129,22 @@ power_get_available_freqs(struct acpi_power_info *pi)\n \tint ret = -1, i, count;\n \tchar *p;\n \tchar buf[BUFSIZ];\n-\tchar fullpath[PATH_MAX];\n \tchar *freqs[RTE_MAX_LCORE_FREQS];\n-\tchar *s;\n-\n-\tsnprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_AVAIL_FREQ,\n-\t\t\tpi->lcore_id);\n-\tf = fopen(fullpath, \"r\");\n-\tFOPEN_OR_ERR_RET(f, ret);\n \n-\ts = fgets(buf, sizeof(buf), f);\n-\tFOPS_OR_NULL_GOTO(s, out);\n+\topen_core_sysfs_file(POWER_SYSFILE_AVAIL_FREQ, pi->lcore_id, \"r\",\n+\t\t\t&f);\n+\tif (f == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_AVAIL_FREQ);\n+\t\tgoto out;\n+\t}\n \n-\t/* Strip the line break if there is */\n-\tp = strchr(buf, '\\n');\n-\tif (p != NULL)\n-\t\t*p = 0;\n+\tret = read_core_sysfs_s(f, buf, sizeof(buf));\n+\tif ((ret) < 0) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to read %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_AVAIL_FREQ);\n+\t\tgoto out;\n+\t}\n \n \t/* Split string into at most RTE_MAX_LCORE_FREQS frequencies */\n \tcount = rte_strsplit(buf, sizeof(buf), freqs,\n@@ -250,7 +184,8 @@ power_get_available_freqs(struct acpi_power_info *pi)\n \tPOWER_DEBUG_TRACE(\"%d frequency(s) of lcore %u are available\\n\",\n \t\t\tcount, pi->lcore_id);\n out:\n-\tfclose(f);\n+\tif (f != NULL)\n+\t\tfclose(f);\n \n \treturn ret;\n }\n@@ -262,18 +197,24 @@ static int\n power_init_for_setting_freq(struct acpi_power_info *pi)\n {\n \tFILE *f;\n-\tchar fullpath[PATH_MAX];\n \tchar buf[BUFSIZ];\n \tuint32_t i, freq;\n-\tchar *s;\n+\tint ret;\n \n-\tsnprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_SETSPEED,\n-\t\t\tpi->lcore_id);\n-\tf = fopen(fullpath, \"rw+\");\n-\tFOPEN_OR_ERR_RET(f, -1);\n+\topen_core_sysfs_file(POWER_SYSFILE_SETSPEED, pi->lcore_id, \"rw+\",\n+\t\t\t&f);\n+\tif (f == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_SETSPEED);\n+\t\tgoto err;\n+\t}\n \n-\ts = fgets(buf, sizeof(buf), f);\n-\tFOPS_OR_NULL_GOTO(s, out);\n+\tret = read_core_sysfs_s(f, buf, sizeof(buf));\n+\tif ((ret) < 0) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to read %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_SETSPEED);\n+\t\tgoto err;\n+\t}\n \n \tfreq = strtoul(buf, NULL, POWER_CONVERT_TO_DECIMAL);\n \tfor (i = 0; i < pi->nb_freqs; i++) {\n@@ -284,8 +225,9 @@ power_init_for_setting_freq(struct acpi_power_info *pi)\n \t\t}\n \t}\n \n-out:\n-\tfclose(f);\n+err:\n+\tif (f != NULL)\n+\t\tfclose(f);\n \n \treturn -1;\n }\n@@ -369,54 +311,6 @@ power_acpi_cpufreq_init(unsigned int lcore_id)\n \treturn -1;\n }\n \n-/**\n- * It is to check the governor and then set the original governor back if\n- * needed by writing the sys file.\n- */\n-static int\n-power_set_governor_original(struct acpi_power_info *pi)\n-{\n-\tFILE *f;\n-\tint ret = -1;\n-\tchar buf[BUFSIZ];\n-\tchar fullpath[PATH_MAX];\n-\tchar *s;\n-\tint val;\n-\n-\tsnprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_GOVERNOR,\n-\t\t\tpi->lcore_id);\n-\tf = fopen(fullpath, \"rw+\");\n-\tFOPEN_OR_ERR_RET(f, ret);\n-\n-\ts = fgets(buf, sizeof(buf), f);\n-\tFOPS_OR_NULL_GOTO(s, out);\n-\n-\t/* Check if the governor to be set is the same as current */\n-\tif (strncmp(buf, pi->governor_ori, sizeof(pi->governor_ori)) == 0) {\n-\t\tret = 0;\n-\t\tPOWER_DEBUG_TRACE(\"Power management governor of lcore %u \"\n-\t\t\t\t\"has already been set to %s\\n\",\n-\t\t\t\tpi->lcore_id, pi->governor_ori);\n-\t\tgoto out;\n-\t}\n-\n-\t/* Write back the original governor */\n-\tval = fseek(f, 0, SEEK_SET);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\tval = fputs(pi->governor_ori, f);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\tret = 0;\n-\tRTE_LOG(INFO, POWER, \"Power management governor of lcore %u \"\n-\t\t\t\"has been set back to %s successfully\\n\",\n-\t\t\tpi->lcore_id, pi->governor_ori);\n-out:\n-\tfclose(f);\n-\n-\treturn ret;\n-}\n-\n int\n power_acpi_cpufreq_exit(unsigned int lcore_id)\n {\ndiff --git a/lib/power/power_common.c b/lib/power/power_common.c\nindex 67e3318ec7..0295b89f10 100644\n--- a/lib/power/power_common.c\n+++ b/lib/power/power_common.c\n@@ -3,13 +3,20 @@\n  */\n \n #include <limits.h>\n+#include <stdlib.h>\n #include <stdio.h>\n #include <string.h>\n \n+#include <rte_log.h>\n+#include <rte_string_fns.h>\n+\n #include \"power_common.h\"\n \n #define POWER_SYSFILE_SCALING_DRIVER   \\\n \t\t\"/sys/devices/system/cpu/cpu%u/cpufreq/scaling_driver\"\n+#define POWER_SYSFILE_GOVERNOR  \\\n+\t\t\"/sys/devices/system/cpu/cpu%u/cpufreq/scaling_governor\"\n+#define POWER_CONVERT_TO_DECIMAL 10\n \n int\n cpufreq_check_scaling_driver(const char *driver_name)\n@@ -58,3 +65,142 @@ cpufreq_check_scaling_driver(const char *driver_name)\n \t */\n \treturn 1;\n }\n+\n+int\n+open_core_sysfs_file(const char *template, unsigned int core, const char *mode,\n+\t\tFILE **f)\n+{\n+\tchar fullpath[PATH_MAX];\n+\tFILE *tmpf;\n+\n+\t/* silenced -Wformat-nonliteral here */\n+\tsnprintf(fullpath, sizeof(fullpath), template, core);\n+\ttmpf = fopen(fullpath, mode);\n+\t*f = tmpf;\n+\tif (tmpf == NULL)\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n+int\n+read_core_sysfs_u32(FILE *f, uint32_t *val)\n+{\n+\tchar buf[BUFSIZ];\n+\tuint32_t fval;\n+\tchar *s;\n+\n+\ts = fgets(buf, sizeof(buf), f);\n+\tif (s == NULL)\n+\t\treturn -1;\n+\n+\t/* fgets puts null terminator in, but do this just in case */\n+\tbuf[BUFSIZ - 1] = '\\0';\n+\n+\t/* strip off any terminating newlines */\n+\t*strchrnul(buf, '\\n') = '\\0';\n+\n+\tfval = strtoul(buf, NULL, POWER_CONVERT_TO_DECIMAL);\n+\n+\t/* write the value */\n+\t*val = fval;\n+\n+\treturn 0;\n+}\n+\n+int\n+read_core_sysfs_s(FILE *f, char *buf, unsigned int len)\n+{\n+\tchar *s;\n+\n+\ts = fgets(buf, len, f);\n+\tif (s == NULL)\n+\t\treturn -1;\n+\n+\t/* fgets puts null terminator in, but do this just in case */\n+\tbuf[len - 1] = '\\0';\n+\n+\t/* strip off any terminating newlines */\n+\t*strchrnul(buf, '\\n') = '\\0';\n+\n+\treturn 0;\n+}\n+\n+int\n+write_core_sysfs_s(FILE *f, const char *str)\n+{\n+\tint ret;\n+\n+\tret = fseek(f, 0, SEEK_SET);\n+\tif (ret != 0)\n+\t\treturn -1;\n+\n+\tret = fputs(str, f);\n+\tif (ret != 0)\n+\t\treturn -1;\n+\n+\t/* flush the output */\n+\tret = fflush(f);\n+\tif (ret != 0)\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n+/**\n+ * It is to check the current scaling governor by reading sys file, and then\n+ * set it into 'performance' if it is not by writing the sys file. The original\n+ * governor will be saved for rolling back.\n+ */\n+int\n+power_set_governor(unsigned int lcore_id, const char *new_governor,\n+\t\tchar *orig_governor, size_t orig_governor_len)\n+{\n+\tFILE *f_governor = NULL;\n+\tint ret = -1;\n+\tchar buf[BUFSIZ];\n+\n+\topen_core_sysfs_file(POWER_SYSFILE_GOVERNOR, lcore_id, \"rw+\",\n+\t\t\t&f_governor);\n+\tif (f_governor == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_GOVERNOR);\n+\t\tgoto out;\n+\t}\n+\n+\tret = read_core_sysfs_s(f_governor, buf, sizeof(buf));\n+\tif (ret < 0) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to read %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_GOVERNOR);\n+\t\tgoto out;\n+\t}\n+\n+\t/* Save the original governor, if it was provided */\n+\tif (orig_governor)\n+\t\trte_strscpy(orig_governor, buf, orig_governor_len);\n+\n+\t/* Check if current governor is already what we want */\n+\tif (strcmp(buf, new_governor) == 0) {\n+\t\tret = 0;\n+\t\tPOWER_DEBUG_TRACE(\"Power management governor of lcore %u is \"\n+\t\t\t\t\"already %s\\n\", lcore_id, new_governor);\n+\t\tgoto out;\n+\t}\n+\n+\t/* Write the new governor */\n+\tret = write_core_sysfs_s(f_governor, new_governor);\n+\tif (ret < 0) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to write %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_GOVERNOR);\n+\t\tgoto out;\n+\t}\n+\n+\tret = 0;\n+\tRTE_LOG(INFO, POWER, \"Power management governor of lcore %u has been \"\n+\t\t\t\"set to '%s' successfully\\n\", lcore_id, new_governor);\n+out:\n+\tif (f_governor != NULL)\n+\t\tfclose(f_governor);\n+\n+\treturn ret;\n+}\ndiff --git a/lib/power/power_common.h b/lib/power/power_common.h\nindex fab3ca995a..ef9f5fa855 100644\n--- a/lib/power/power_common.h\n+++ b/lib/power/power_common.h\n@@ -5,9 +5,26 @@\n #ifndef _POWER_COMMON_H_\n #define _POWER_COMMON_H_\n \n+#include <inttypes.h>\n+\n #define RTE_POWER_INVALID_FREQ_INDEX (~0)\n \n+\n+#ifdef RTE_LIBRTE_POWER_DEBUG\n+#define POWER_DEBUG_TRACE(fmt, args...) \\\n+\t\tRTE_LOG(ERR, POWER, \"%s: \" fmt, __func__, ## args)\n+#else\n+#define POWER_DEBUG_TRACE(fmt, args...)\n+#endif\n+\n /* check if scaling driver matches one we want */\n int cpufreq_check_scaling_driver(const char *driver);\n+int power_set_governor(unsigned int lcore_id, const char *new_governor,\n+\t\tchar *orig_governor, size_t orig_governor_len);\n+int open_core_sysfs_file(const char *template, unsigned int core,\n+\t\t\t const char *mode, FILE **f);\n+int read_core_sysfs_u32(FILE *f, uint32_t *val);\n+int read_core_sysfs_s(FILE *f, char *buf, unsigned int len);\n+int write_core_sysfs_s(FILE *f, const char *str);\n \n #endif /* _POWER_COMMON_H_ */\ndiff --git a/lib/power/power_pstate_cpufreq.c b/lib/power/power_pstate_cpufreq.c\nindex 2cfc54acf3..ed10bd193b 100644\n--- a/lib/power/power_pstate_cpufreq.c\n+++ b/lib/power/power_pstate_cpufreq.c\n@@ -21,46 +21,13 @@\n #include \"power_pstate_cpufreq.h\"\n #include \"power_common.h\"\n \n-\n-#ifdef RTE_LIBRTE_POWER_DEBUG\n-#define POWER_DEBUG_TRACE(fmt, args...) do { \\\n-\t\tRTE_LOG(ERR, POWER, \"%s: \" fmt, __func__, ## args); \\\n-} while (0)\n-#else\n-#define POWER_DEBUG_TRACE(fmt, args...)\n-#endif\n-\n-#define FOPEN_OR_ERR_RET(f, retval) do { \\\n-\t\tif ((f) == NULL) { \\\n-\t\t\tRTE_LOG(ERR, POWER, \"File not opened\\n\"); \\\n-\t\t\treturn retval; \\\n-\t\t} \\\n-} while (0)\n-\n-#define FOPS_OR_NULL_GOTO(ret, label) do { \\\n-\t\tif ((ret) == NULL) { \\\n-\t\t\tRTE_LOG(ERR, POWER, \"fgets returns nothing\\n\"); \\\n-\t\t\tgoto label; \\\n-\t\t} \\\n-} while (0)\n-\n-#define FOPS_OR_ERR_GOTO(ret, label) do { \\\n-\t\tif ((ret) < 0) { \\\n-\t\t\tRTE_LOG(ERR, POWER, \"File operations failed\\n\"); \\\n-\t\t\tgoto label; \\\n-\t\t} \\\n-} while (0)\n-\n /* macros used for rounding frequency to nearest 100000 */\n #define FREQ_ROUNDING_DELTA 50000\n #define ROUND_FREQ_TO_N_100000 100000\n \n-#define POWER_CONVERT_TO_DECIMAL 10\n #define BUS_FREQ     100000\n \n #define POWER_GOVERNOR_PERF \"performance\"\n-#define POWER_SYSFILE_GOVERNOR  \\\n-\t\t\"/sys/devices/system/cpu/cpu%u/cpufreq/scaling_governor\"\n #define POWER_SYSFILE_MAX_FREQ \\\n \t\t\"/sys/devices/system/cpu/cpu%u/cpufreq/scaling_max_freq\"\n #define POWER_SYSFILE_MIN_FREQ  \\\n@@ -154,91 +121,78 @@ out:\tclose(fd);\n static int\n power_init_for_setting_freq(struct pstate_power_info *pi)\n {\n-\tFILE *f_min, *f_max, *f_base = NULL, *f_base_max;\n-\tchar fullpath_min[PATH_MAX];\n-\tchar fullpath_max[PATH_MAX];\n-\tchar fullpath_base[PATH_MAX];\n-\tchar fullpath_base_max[PATH_MAX];\n-\tchar buf_base[BUFSIZ];\n-\tchar *s_base;\n-\tchar *s_base_max;\n-\tuint32_t base_ratio = 0;\n-\tuint32_t base_max_ratio = 0;\n-\tuint64_t max_non_turbo = 0;\n-\tint  ret_val = 0;\n-\n-\tsnprintf(fullpath_base_max,\n-\t\t\tsizeof(fullpath_base_max),\n-\t\t\tPOWER_SYSFILE_BASE_MAX_FREQ,\n-\t\t\tpi->lcore_id);\n-\tf_base_max = fopen(fullpath_base_max, \"r\");\n-\tFOPEN_OR_ERR_RET(f_base_max, -1);\n-\tif (f_base_max != NULL) {\n-\t\ts_base_max = fgets(buf_base, sizeof(buf_base), f_base_max);\n-\n-\t\t/* close the file unconditionally */\n-\t\tfclose(f_base_max);\n-\t\tf_base_max = NULL;\n-\n-\t\tFOPS_OR_NULL_GOTO(s_base_max, out);\n-\n-\t\tbuf_base[BUFSIZ-1] = '\\0';\n-\t\tif (strlen(buf_base))\n-\t\t\t/* Strip off terminating '\\n' */\n-\t\t\tstrtok(buf_base, \"\\n\");\n-\n-\t\tbase_max_ratio =\n-\t\t\tstrtoul(buf_base, NULL, POWER_CONVERT_TO_DECIMAL)\n-\t\t\t\t/ BUS_FREQ;\n+\tFILE *f_base = NULL, *f_base_max = NULL, *f_min = NULL, *f_max = NULL;\n+\tuint32_t base_ratio, base_max_ratio;\n+\tuint64_t max_non_turbo;\n+\tint ret;\n+\n+\t/* open all files we expect to have open */\n+\topen_core_sysfs_file(POWER_SYSFILE_BASE_MAX_FREQ, pi->lcore_id, \"r\",\n+\t\t\t&f_base_max);\n+\tif (f_base_max == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_BASE_MAX_FREQ);\n+\t\tgoto err;\n+\t}\n+\n+\topen_core_sysfs_file(POWER_SYSFILE_MIN_FREQ, pi->lcore_id, \"rw+\",\n+\t\t\t&f_min);\n+\tif (f_min == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_MIN_FREQ);\n+\t\tgoto err;\n+\t}\n+\n+\topen_core_sysfs_file(POWER_SYSFILE_MAX_FREQ, pi->lcore_id, \"rw+\",\n+\t\t\t&f_max);\n+\tif (f_max == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_MAX_FREQ);\n+\t\tgoto err;\n+\t}\n+\n+\topen_core_sysfs_file(POWER_SYSFILE_BASE_FREQ, pi->lcore_id, \"r\",\n+\t\t\t&f_base);\n+\t/* base ratio file may not exist in some kernels, so no error check */\n+\n+\t/* read base max ratio */\n+\tret = read_core_sysfs_u32(f_base_max, &base_max_ratio);\n+\tif (ret < 0) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to read %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_BASE_MAX_FREQ);\n+\t\tgoto err;\n+\t}\n+\n+\t/* base ratio may not exist */\n+\tif (f_base != NULL) {\n+\t\tret = read_core_sysfs_u32(f_base, &base_ratio);\n+\t\tif (ret < 0) {\n+\t\t\tRTE_LOG(ERR, POWER, \"Failed to read %s\\n\",\n+\t\t\t\t\tPOWER_SYSFILE_BASE_FREQ);\n+\t\t\tgoto err;\n+\t\t}\n+\t} else {\n+\t\tbase_ratio = 0;\n \t}\n \n-\tsnprintf(fullpath_min, sizeof(fullpath_min), POWER_SYSFILE_MIN_FREQ,\n-\t\t\tpi->lcore_id);\n-\tf_min = fopen(fullpath_min, \"rw+\");\n-\tFOPEN_OR_ERR_RET(f_min, -1);\n+\t/* Add MSR read to detect turbo status */\n+\tif (power_rdmsr(PLATFORM_INFO, &max_non_turbo, pi->lcore_id) < 0)\n+\t\tgoto err;\n+\t/* no errors after this point */\n \n-\tsnprintf(fullpath_max, sizeof(fullpath_max), POWER_SYSFILE_MAX_FREQ,\n-\t\t\tpi->lcore_id);\n-\tf_max = fopen(fullpath_max, \"rw+\");\n-\tif (f_max == NULL)\n-\t\tfclose(f_min);\n-\tFOPEN_OR_ERR_RET(f_max, -1);\n+\t/* convert ratios to bins */\n+\tbase_max_ratio /= BUS_FREQ;\n+\tbase_ratio /= BUS_FREQ;\n \n+\t/* assign file handles */\n \tpi->f_cur_min = f_min;\n \tpi->f_cur_max = f_max;\n \n-\tsnprintf(fullpath_base, sizeof(fullpath_base), POWER_SYSFILE_BASE_FREQ,\n-\t\t\tpi->lcore_id);\n-\n-\tf_base = fopen(fullpath_base, \"r\");\n-\tif (f_base == NULL) {\n-\t\t/* No sysfs base_frequency, that's OK, continue without */\n-\t\tbase_ratio = 0;\n-\t} else {\n-\t\ts_base = fgets(buf_base, sizeof(buf_base), f_base);\n-\t\tFOPS_OR_NULL_GOTO(s_base, out);\n-\n-\t\tbuf_base[BUFSIZ-1] = '\\0';\n-\t\tif (strlen(buf_base))\n-\t\t\t/* Strip off terminating '\\n' */\n-\t\t\tstrtok(buf_base, \"\\n\");\n-\n-\t\tbase_ratio = strtoul(buf_base, NULL, POWER_CONVERT_TO_DECIMAL)\n-\t\t\t\t/ BUS_FREQ;\n-\t}\n-\n-\t/* Add MSR read to detect turbo status */\n-\n-\tif (power_rdmsr(PLATFORM_INFO, &max_non_turbo, pi->lcore_id) < 0) {\n-\t\tret_val = -1;\n-\t\tgoto out;\n-\t}\n-\n \tmax_non_turbo = (max_non_turbo&NON_TURBO_MASK)>>NON_TURBO_OFFSET;\n \n \tPOWER_DEBUG_TRACE(\"no turbo perf %\"PRIu64\"\\n\", max_non_turbo);\n \n-\tpi->non_turbo_max_ratio = max_non_turbo;\n+\tpi->non_turbo_max_ratio = (uint32_t)max_non_turbo;\n \n \t/*\n \t * If base_frequency is reported as greater than the maximum\n@@ -264,7 +218,20 @@ power_init_for_setting_freq(struct pstate_power_info *pi)\n out:\n \tif (f_base != NULL)\n \t\tfclose(f_base);\n-\treturn ret_val;\n+\tfclose(f_base_max);\n+\t/* f_min and f_max are stored, no need to close */\n+\treturn 0;\n+\n+err:\n+\tif (f_base != NULL)\n+\t\tfclose(f_base);\n+\tif (f_base_max != NULL)\n+\t\tfclose(f_base_max);\n+\tif (f_min != NULL)\n+\t\tfclose(f_min);\n+\tif (f_max != NULL)\n+\t\tfclose(f_max);\n+\treturn -1;\n }\n \n static int\n@@ -369,53 +336,8 @@ set_freq_internal(struct pstate_power_info *pi, uint32_t idx)\n static int\n power_set_governor_performance(struct pstate_power_info *pi)\n {\n-\tFILE *f;\n-\tint ret = -1;\n-\tchar buf[BUFSIZ];\n-\tchar fullpath[PATH_MAX];\n-\tchar *s;\n-\tint val;\n-\n-\tsnprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_GOVERNOR,\n-\t\t\tpi->lcore_id);\n-\tf = fopen(fullpath, \"rw+\");\n-\tFOPEN_OR_ERR_RET(f, ret);\n-\n-\ts = fgets(buf, sizeof(buf), f);\n-\tFOPS_OR_NULL_GOTO(s, out);\n-\t/* Strip off terminating '\\n' */\n-\tstrtok(buf, \"\\n\");\n-\n-\t/* Save the original governor */\n-\trte_strscpy(pi->governor_ori, buf, sizeof(pi->governor_ori));\n-\n-\t/* Check if current governor is performance */\n-\tif (strncmp(buf, POWER_GOVERNOR_PERF,\n-\t\t\tsizeof(POWER_GOVERNOR_PERF)) == 0) {\n-\t\tret = 0;\n-\t\tPOWER_DEBUG_TRACE(\"Power management governor of lcore %u is \"\n-\t\t\t\t\"already performance\\n\", pi->lcore_id);\n-\t\tgoto out;\n-\t}\n-\n-\t/* Write 'performance' to the governor */\n-\tval = fseek(f, 0, SEEK_SET);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\tval = fputs(POWER_GOVERNOR_PERF, f);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\t/* We need to flush to see if the fputs succeeds */\n-\tval = fflush(f);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\tret = 0;\n-\tRTE_LOG(INFO, POWER, \"Power management governor of lcore %u has been \"\n-\t\t\t\"set to performance successfully\\n\", pi->lcore_id);\n-out:\n-\tfclose(f);\n-\n-\treturn ret;\n+\treturn power_set_governor(pi->lcore_id, POWER_GOVERNOR_PERF,\n+\t\t\tpi->governor_ori, sizeof(pi->governor_ori));\n }\n \n /**\n@@ -425,45 +347,7 @@ power_set_governor_performance(struct pstate_power_info *pi)\n static int\n power_set_governor_original(struct pstate_power_info *pi)\n {\n-\tFILE *f;\n-\tint ret = -1;\n-\tchar buf[BUFSIZ];\n-\tchar fullpath[PATH_MAX];\n-\tchar *s;\n-\tint val;\n-\n-\tsnprintf(fullpath, sizeof(fullpath), POWER_SYSFILE_GOVERNOR,\n-\t\t\tpi->lcore_id);\n-\tf = fopen(fullpath, \"rw+\");\n-\tFOPEN_OR_ERR_RET(f, ret);\n-\n-\ts = fgets(buf, sizeof(buf), f);\n-\tFOPS_OR_NULL_GOTO(s, out);\n-\n-\t/* Check if the governor to be set is the same as current */\n-\tif (strncmp(buf, pi->governor_ori, sizeof(pi->governor_ori)) == 0) {\n-\t\tret = 0;\n-\t\tPOWER_DEBUG_TRACE(\"Power management governor of lcore %u \"\n-\t\t\t\t\"has already been set to %s\\n\",\n-\t\t\t\tpi->lcore_id, pi->governor_ori);\n-\t\tgoto out;\n-\t}\n-\n-\t/* Write back the original governor */\n-\tval = fseek(f, 0, SEEK_SET);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\tval = fputs(pi->governor_ori, f);\n-\tFOPS_OR_ERR_GOTO(val, out);\n-\n-\tret = 0;\n-\tRTE_LOG(INFO, POWER, \"Power management governor of lcore %u \"\n-\t\t\t\"has been set back to %s successfully\\n\",\n-\t\t\tpi->lcore_id, pi->governor_ori);\n-out:\n-\tfclose(f);\n-\n-\treturn ret;\n+\treturn power_set_governor(pi->lcore_id, pi->governor_ori, NULL, 0);\n }\n \n /**\n@@ -473,51 +357,42 @@ power_set_governor_original(struct pstate_power_info *pi)\n static int\n power_get_available_freqs(struct pstate_power_info *pi)\n {\n-\tFILE *f_min, *f_max;\n+\tFILE *f_min = NULL, *f_max = NULL;\n \tint ret = -1;\n-\tchar *p_min, *p_max;\n-\tchar buf_min[BUFSIZ];\n-\tchar buf_max[BUFSIZ];\n-\tchar fullpath_min[PATH_MAX];\n-\tchar fullpath_max[PATH_MAX];\n-\tchar *s_min, *s_max;\n \tuint32_t sys_min_freq = 0, sys_max_freq = 0, base_max_freq = 0;\n \tuint32_t i, num_freqs = 0;\n \n-\tsnprintf(fullpath_max, sizeof(fullpath_max),\n-\t\t\tPOWER_SYSFILE_BASE_MAX_FREQ,\n-\t\t\tpi->lcore_id);\n-\tsnprintf(fullpath_min, sizeof(fullpath_min),\n-\t\t\tPOWER_SYSFILE_BASE_MIN_FREQ,\n-\t\t\tpi->lcore_id);\n-\n-\tf_min = fopen(fullpath_min, \"r\");\n-\tFOPEN_OR_ERR_RET(f_min, ret);\n-\n-\tf_max = fopen(fullpath_max, \"r\");\n-\tif (f_max == NULL)\n-\t\tfclose(f_min);\n-\n-\tFOPEN_OR_ERR_RET(f_max, ret);\n-\n-\ts_min = fgets(buf_min, sizeof(buf_min), f_min);\n-\tFOPS_OR_NULL_GOTO(s_min, out);\n-\n-\ts_max = fgets(buf_max, sizeof(buf_max), f_max);\n-\tFOPS_OR_NULL_GOTO(s_max, out);\n-\n+\t/* open all files */\n+\topen_core_sysfs_file(POWER_SYSFILE_BASE_MAX_FREQ, pi->lcore_id, \"r\",\n+\t\t\t&f_max);\n+\tif (f_max == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_BASE_MAX_FREQ);\n+\t\tgoto out;\n+\t}\n \n-\t/* Strip the line break if there is */\n-\tp_min = strchr(buf_min, '\\n');\n-\tif (p_min != NULL)\n-\t\t*p_min = 0;\n+\topen_core_sysfs_file(POWER_SYSFILE_BASE_MIN_FREQ, pi->lcore_id, \"r\",\n+\t\t\t     &f_min);\n+\tif (f_min == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_BASE_MIN_FREQ);\n+\t\tgoto out;\n+\t}\n \n-\tp_max = strchr(buf_max, '\\n');\n-\tif (p_max != NULL)\n-\t\t*p_max = 0;\n+\t/* read base ratios */\n+\tret = read_core_sysfs_u32(f_max, &sys_max_freq);\n+\tif (ret < 0) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to read %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_BASE_MAX_FREQ);\n+\t\tgoto out;\n+\t}\n \n-\tsys_min_freq = strtoul(buf_min, &p_min, POWER_CONVERT_TO_DECIMAL);\n-\tsys_max_freq = strtoul(buf_max, &p_max, POWER_CONVERT_TO_DECIMAL);\n+\tret = read_core_sysfs_u32(f_min, &sys_min_freq);\n+\tif (ret < 0) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to read %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_BASE_MIN_FREQ);\n+\t\tgoto out;\n+\t}\n \n \tif (sys_max_freq < sys_min_freq)\n \t\tgoto out;\n@@ -576,27 +451,22 @@ power_get_cur_idx(struct pstate_power_info *pi)\n {\n \tFILE *f_cur;\n \tint ret = -1;\n-\tchar *p_cur;\n-\tchar buf_cur[BUFSIZ];\n-\tchar fullpath_cur[PATH_MAX];\n-\tchar *s_cur;\n \tuint32_t sys_cur_freq = 0;\n \tunsigned int i;\n \n-\tsnprintf(fullpath_cur, sizeof(fullpath_cur),\n-\t\t\tPOWER_SYSFILE_CUR_FREQ,\n-\t\t\tpi->lcore_id);\n-\tf_cur = fopen(fullpath_cur, \"r\");\n-\tFOPEN_OR_ERR_RET(f_cur, ret);\n-\n-\t/* initialize the cur_idx to matching current frequency freq index */\n-\ts_cur = fgets(buf_cur, sizeof(buf_cur), f_cur);\n-\tFOPS_OR_NULL_GOTO(s_cur, fail);\n+\topen_core_sysfs_file(POWER_SYSFILE_CUR_FREQ, pi->lcore_id, \"r\", &f_cur);\n+\tif (f_cur == NULL) {\n+\t\tRTE_LOG(ERR, POWER, \"failed to open %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_CUR_FREQ);\n+\t\tgoto fail;\n+\t}\n \n-\tp_cur = strchr(buf_cur, '\\n');\n-\tif (p_cur != NULL)\n-\t\t*p_cur = 0;\n-\tsys_cur_freq = strtoul(buf_cur, &p_cur, POWER_CONVERT_TO_DECIMAL);\n+\tret = read_core_sysfs_u32(f_cur, &sys_cur_freq);\n+\tif (ret < 0) {\n+\t\tRTE_LOG(ERR, POWER, \"Failed to read %s\\n\",\n+\t\t\t\tPOWER_SYSFILE_CUR_FREQ);\n+\t\tgoto fail;\n+\t}\n \n \t/* convert the frequency to nearest 100000 value\n \t * Ex: if sys_cur_freq=1396789 then freq_conv=1400000\n@@ -615,10 +485,10 @@ power_get_cur_idx(struct pstate_power_info *pi)\n \t\t}\n \t}\n \n-\tfclose(f_cur);\n-\treturn 0;\n+\tret = 0;\n fail:\n-\tfclose(f_cur);\n+\tif (f_cur != NULL)\n+\t\tfclose(f_cur);\n \treturn ret;\n }\n \n",
    "prefixes": [
        "v5",
        "2/2"
    ]
}