Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/90110/?format=api
{ "id": 90110, "url": "http://patchwork.dpdk.org/api/patches/90110/?format=api", "web_url": "http://patchwork.dpdk.org/project/dpdk/patch/d0dc49e5dab4d4c730cc137194350a7d0a8baca8.1617113740.git.anatoly.burakov@intel.com/", "project": { "id": 1, "url": "http://patchwork.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": "<d0dc49e5dab4d4c730cc137194350a7d0a8baca8.1617113740.git.anatoly.burakov@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/d0dc49e5dab4d4c730cc137194350a7d0a8baca8.1617113740.git.anatoly.burakov@intel.com", "date": "2021-03-30T14:15:46", "name": "[1/3] power: refactor base frequency detection", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "89527363bdf751840b8a55baa06bf85a0265ccd3", "submitter": { "id": 4, "url": "http://patchwork.dpdk.org/api/people/4/?format=api", "name": "Burakov, Anatoly", "email": "anatoly.burakov@intel.com" }, "delegate": null, "mbox": "http://patchwork.dpdk.org/project/dpdk/patch/d0dc49e5dab4d4c730cc137194350a7d0a8baca8.1617113740.git.anatoly.burakov@intel.com/mbox/", "series": [ { "id": 15983, "url": "http://patchwork.dpdk.org/api/series/15983/?format=api", "web_url": "http://patchwork.dpdk.org/project/dpdk/list/?series=15983", "date": "2021-03-30T14:15:46", "name": "[1/3] power: refactor base frequency detection", "version": 1, "mbox": "http://patchwork.dpdk.org/series/15983/mbox/" } ], "comments": "http://patchwork.dpdk.org/api/patches/90110/comments/", "check": "warning", "checks": "http://patchwork.dpdk.org/api/patches/90110/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 DA73CA034F;\n\tTue, 30 Mar 2021 16:15:55 +0200 (CEST)", "from [217.70.189.124] (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id CAE29140E02;\n\tTue, 30 Mar 2021 16:15:55 +0200 (CEST)", "from mga14.intel.com (mga14.intel.com [192.55.52.115])\n by mails.dpdk.org (Postfix) with ESMTP id 2112140691\n for <dev@dpdk.org>; Tue, 30 Mar 2021 16:15:51 +0200 (CEST)", "from orsmga006.jf.intel.com ([10.7.209.51])\n by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n 30 Mar 2021 07:15:50 -0700", "from silpixa00399498.ir.intel.com (HELO\n silpixa00399498.ger.corp.intel.com) ([10.237.222.97])\n by orsmga006.jf.intel.com with ESMTP; 30 Mar 2021 07:15:49 -0700" ], "IronPort-SDR": [ "\n Fu41VQHY78a0JmWLPU8cp+lU1iiqITJVL4YX0uYY2/gQof2Dcd0aOYTtcXgYWAZWnKZYPUXI2K\n RI2CBkOQihHA==", "\n 9eUer95M33YwcvDCpM1kiwz6RO7bZUjAKD+EKseoTBsETpDPmhRl8Tat/hyEWFY2JFv5iaDdPn\n L6r1b1kYMMMQ==" ], "X-IronPort-AV": [ "E=McAfee;i=\"6000,8403,9939\"; a=\"191249257\"", "E=Sophos;i=\"5.81,291,1610438400\"; d=\"scan'208\";a=\"191249257\"", "E=Sophos;i=\"5.81,291,1610438400\"; d=\"scan'208\";a=\"378506442\"" ], "X-ExtLoop1": "1", "From": "Anatoly Burakov <anatoly.burakov@intel.com>", "To": "dev@dpdk.org", "Cc": "David Hunt <david.hunt@intel.com>", "Date": "Tue, 30 Mar 2021 14:15:46 +0000", "Message-Id": "\n <d0dc49e5dab4d4c730cc137194350a7d0a8baca8.1617113740.git.anatoly.burakov@intel.com>", "X-Mailer": "git-send-email 2.25.1", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[dpdk-dev] [PATCH 1/3] power: refactor base frequency detection", "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": "Currently, base frequency detection code is a bit of an unmaintainable\nmess that has a couple of Coverity issues (dead code, and a resource\nleak). Rather than just fixing the issues and leaving the rest in place,\nrefactor the code in a way that makes it more maintainable and less\nprone to such Coverity issues in the first place.\n\nCoverity issue: 369693\nCoverity issue: 369694\n\nFixes: 4db9587bbf72 (\"power: check sysfs base frequency\")\nCc: david.hunt@intel.com\n\nSigned-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\n---\n lib/librte_power/meson.build | 7 +\n lib/librte_power/power_pstate_cpufreq.c | 178 ++++++++++++++----------\n 2 files changed, 113 insertions(+), 72 deletions(-)", "diff": "diff --git a/lib/librte_power/meson.build b/lib/librte_power/meson.build\nindex 9a2dcbfc7a..fd408ffd4c 100644\n--- a/lib/librte_power/meson.build\n+++ b/lib/librte_power/meson.build\n@@ -5,6 +5,13 @@ if not is_linux\n \tbuild = false\n \treason = '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('rte_power.c', 'power_acpi_cpufreq.c',\n \t\t'power_kvm_vm.c', 'guest_channel.c',\n \t\t'rte_power_empty_poll.c',\ndiff --git a/lib/librte_power/power_pstate_cpufreq.c b/lib/librte_power/power_pstate_cpufreq.c\nindex 8a1fffaed5..add06720db 100644\n--- a/lib/librte_power/power_pstate_cpufreq.c\n+++ b/lib/librte_power/power_pstate_cpufreq.c\n@@ -37,6 +37,13 @@\n \t\t} \\\n } while (0)\n \n+#define FOPEN_OR_ERR_GOTO(f, label) do { \\\n+\t\tif ((f) == NULL) { \\\n+\t\t\tRTE_LOG(ERR, POWER, \"File not opened\\n\"); \\\n+\t\t\tgoto label; \\\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@@ -148,93 +155,107 @@ out:\tclose(fd);\n \treturn ret;\n }\n \n+static 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+\tif (tmpf == NULL)\n+\t\treturn -1;\n+\t*f = tmpf;\n+\n+\treturn 0;\n+}\n+\n+static 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+\tif (strlen(buf))\n+\t\tstrtok(buf, \"\\n\");\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 /**\n * It is to fopen the sys file for the future setting the lcore frequency.\n */\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-\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-\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-\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-\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-\tFOPEN_OR_ERR_RET(f_base, -1);\n-\tif (f_base == NULL) {\n-\t\t/* No sysfs base_frequency, that's OK, continue without */\n-\t\tbase_ratio = 0;\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+\tFOPEN_OR_ERR_GOTO(f_base_max, err);\n+\n+\topen_core_sysfs_file(POWER_SYSFILE_MIN_FREQ, pi->lcore_id, \"rw+\",\n+\t\t\t&f_min);\n+\tFOPEN_OR_ERR_GOTO(f_min, err);\n+\n+\topen_core_sysfs_file(POWER_SYSFILE_MAX_FREQ, pi->lcore_id, \"rw+\",\n+\t\t\t&f_max);\n+\tFOPEN_OR_ERR_GOTO(f_max, err);\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+\tFOPS_OR_ERR_GOTO(ret, err);\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\tFOPS_OR_ERR_GOTO(ret, err);\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\tbase_ratio = 0;\n \t}\n \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-\tif (power_rdmsr(PLATFORM_INFO, &max_non_turbo, pi->lcore_id) < 0) {\n-\t\tret_val = -1;\n-\t\tgoto out;\n-\t}\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 \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@@ -260,7 +281,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", "prefixes": [ "1/3" ] }