From: Fenghua Yu <fenghua.yu@intel.com>
To: "Shuah Khan" <shuah@kernel.org>,
"Tony Luck" <tony.luck@intel.com>,
"Reinette Chatre" <reinette.chatre@intel.com>,
"David Binderman" <dcb314@hotmail.com>,
"Babu Moger" <babu.moger@amd.com>,
"James Morse" <james.morse@arm.com>,
"Ravi V Shankar" <ravi.v.shankar@intel.com>
Cc: "linux-kernel" <linux-kernel@vger.kernel.org>,
Fenghua Yu <fenghua.yu@intel.com>,
Dan Carpenter <dan.carpenter@oracle.com>
Subject: [PATCH v4 17/17] selftests/resctrl: Fix checking for < 0 for unsigned values
Date: Mon, 30 Nov 2020 20:20:10 +0000 [thread overview]
Message-ID: <20201130202010.178373-18-fenghua.yu@intel.com> (raw)
In-Reply-To: <20201130202010.178373-1-fenghua.yu@intel.com>
Dan reported following static checker warnings
tools/testing/selftests/resctrl/resctrl_val.c:545 measure_vals()
warn: 'bw_imc' unsigned <= 0
tools/testing/selftests/resctrl/resctrl_val.c:549 measure_vals()
warn: 'bw_resc_end' unsigned <= 0
These warnings are reported because
1. measure_vals() declares 'bw_imc' and 'bw_resc_end' as unsigned long
variables
2. Return value of get_mem_bw_imc() and get_mem_bw_resctrl() are assigned
to 'bw_imc' and 'bw_resc_end' respectively
3. The returned values are checked for <= 0 to see if the calls failed
Checking for < 0 for an unsigned value doesn't make any sense.
Fix this issue by changing the implementation of get_mem_bw_imc() and
get_mem_bw_resctrl() such that they now accept reference to a variable
and set the variable appropriately upon success and return 0, else return
< 0 on error.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/resctrl_val.c | 41 +++++++++++--------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index d6f0688182e8..ce8f0ec15f7b 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -300,9 +300,9 @@ static int initialize_mem_bw_imc(void)
* Memory B/W utilized by a process on a socket can be calculated using
* iMC counters. Perf events are used to read these counters.
*
- * Return: >= 0 on success. < 0 on failure.
+ * Return: = 0 on success. < 0 on failure.
*/
-static float get_mem_bw_imc(int cpu_no, char *bw_report)
+static int get_mem_bw_imc(int cpu_no, char *bw_report, float *bw_imc)
{
float reads, writes, of_mul_read, of_mul_write;
int imc, j, ret;
@@ -373,13 +373,18 @@ static float get_mem_bw_imc(int cpu_no, char *bw_report)
close(imc_counters_config[imc][WRITE].fd);
}
- if (strcmp(bw_report, "reads") == 0)
- return reads;
+ if (strcmp(bw_report, "reads") == 0) {
+ *bw_imc = reads;
+ return 0;
+ }
- if (strcmp(bw_report, "writes") == 0)
- return writes;
+ if (strcmp(bw_report, "writes") == 0) {
+ *bw_imc = writes;
+ return 0;
+ }
- return (reads + writes);
+ *bw_imc = reads + writes;
+ return 0;
}
void set_mbm_path(const char *ctrlgrp, const char *mongrp, int resource_id)
@@ -438,9 +443,8 @@ static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char *mongrp,
* 1. If con_mon grp is given, then read from it
* 2. If con_mon grp is not given, then read from root con_mon grp
*/
-static unsigned long get_mem_bw_resctrl(void)
+static int get_mem_bw_resctrl(unsigned long *mbm_total)
{
- unsigned long mbm_total = 0;
FILE *fp;
fp = fopen(mbm_total_path, "r");
@@ -449,7 +453,7 @@ static unsigned long get_mem_bw_resctrl(void)
return -1;
}
- if (fscanf(fp, "%lu", &mbm_total) <= 0) {
+ if (fscanf(fp, "%lu", mbm_total) <= 0) {
perror("Could not get mbm local bytes");
fclose(fp);
@@ -457,7 +461,7 @@ static unsigned long get_mem_bw_resctrl(void)
}
fclose(fp);
- return mbm_total;
+ return 0;
}
pid_t bm_pid, ppid;
@@ -549,7 +553,8 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
static int
measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start)
{
- unsigned long bw_imc, bw_resc, bw_resc_end;
+ unsigned long bw_resc, bw_resc_end;
+ float bw_imc;
int ret;
/*
@@ -559,13 +564,13 @@ measure_vals(struct resctrl_val_param *param, unsigned long *bw_resc_start)
* Compare the two values to validate resctrl value.
* It takes 1sec to measure the data.
*/
- bw_imc = get_mem_bw_imc(param->cpu_no, param->bw_report);
- if (bw_imc <= 0)
- return bw_imc;
+ ret = get_mem_bw_imc(param->cpu_no, param->bw_report, &bw_imc);
+ if (ret < 0)
+ return ret;
- bw_resc_end = get_mem_bw_resctrl();
- if (bw_resc_end <= 0)
- return bw_resc_end;
+ ret = get_mem_bw_resctrl(&bw_resc_end);
+ if (ret < 0)
+ return ret;
bw_resc = (bw_resc_end - *bw_resc_start) / MB;
ret = print_results_bw(param->filename, bm_pid, bw_imc, bw_resc);
--
2.29.2
next prev parent reply other threads:[~2020-11-30 20:21 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-30 20:19 [PATCH v4 00/17] Miscellaneous fixes for resctrl selftests Fenghua Yu
2020-11-30 20:19 ` [PATCH v4 01/17] selftests/resctrl: Fix compilation issues for global variables Fenghua Yu
2021-01-26 1:22 ` Shuah Khan
2020-11-30 20:19 ` [PATCH v4 02/17] selftests/resctrl: Clean up resctrl features check Fenghua Yu
2021-01-26 2:08 ` Shuah Khan
2020-11-30 20:19 ` [PATCH v4 03/17] selftests/resctrl: Rename CQM test as CMT test Fenghua Yu
2020-11-30 20:19 ` [PATCH v4 04/17] selftests/resctrl: Fix printed messages Fenghua Yu
2021-01-26 2:25 ` Shuah Khan
2020-11-30 20:19 ` [PATCH v4 05/17] selftests/resctrl: Add a few dependencies Fenghua Yu
2021-01-26 2:29 ` Shuah Khan
2020-11-30 20:19 ` [PATCH v4 06/17] selftests/resctrl: Check for resctrl mount point only if resctrl FS is supported Fenghua Yu
2021-01-26 2:32 ` Shuah Khan
2020-11-30 20:20 ` [PATCH v4 07/17] selftests/resctrl: Use resctrl/info for feature detection Fenghua Yu
2020-11-30 20:20 ` [PATCH v4 08/17] selftests/resctrl: Ensure sibling CPU is not same as original CPU Fenghua Yu
2021-01-26 2:35 ` Shuah Khan
2020-11-30 20:20 ` [PATCH v4 09/17] selftests/resctrl: Fix missing options "-n" and "-p" Fenghua Yu
2021-01-26 2:36 ` Shuah Khan
2020-11-30 20:20 ` [PATCH v4 10/17] selftests/resctrl: Fix MBA/MBM results reporting format Fenghua Yu
2021-01-26 2:38 ` Shuah Khan
2020-11-30 20:20 ` [PATCH v4 11/17] selftests/resctrl: Enable gcc checks to detect buffer overflows Fenghua Yu
2021-01-26 2:38 ` Shuah Khan
2020-11-30 20:20 ` [PATCH v4 12/17] selftests/resctrl: Don't hard code value of "no_of_bits" variable Fenghua Yu
2020-11-30 20:20 ` [PATCH v4 13/17] selftests/resctrl: Modularize resctrl test suite main() function Fenghua Yu
2020-11-30 20:20 ` [PATCH v4 14/17] selftests/resctrl: Skip the test if requested resctrl feature is not supported Fenghua Yu
2020-11-30 20:20 ` [PATCH v4 15/17] selftests/resctrl: Fix unmount resctrl FS Fenghua Yu
2020-11-30 20:20 ` [PATCH v4 16/17] selftests/resctrl: Fix incorrect parsing of iMC counters Fenghua Yu
2020-11-30 20:20 ` Fenghua Yu [this message]
2020-12-11 0:21 ` [PATCH v4 00/17] Miscellaneous fixes for resctrl selftests Yu, Fenghua
2021-01-25 20:47 ` Fenghua Yu
2021-01-25 21:52 ` Shuah Khan
2021-01-25 21:54 ` Fenghua Yu
2021-01-26 1:22 ` Shuah Khan
2021-01-26 23:57 ` Shuah Khan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201130202010.178373-18-fenghua.yu@intel.com \
--to=fenghua.yu@intel.com \
--cc=babu.moger@amd.com \
--cc=dan.carpenter@oracle.com \
--cc=dcb314@hotmail.com \
--cc=james.morse@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=ravi.v.shankar@intel.com \
--cc=reinette.chatre@intel.com \
--cc=shuah@kernel.org \
--cc=tony.luck@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox