All of lore.kernel.org
 help / color / mirror / Atom feed
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>,
	"Babu Moger" <babu.moger@amd.com>
Cc: "linux-kselftest" <linux-kselftest@vger.kernel.org>,
	"linux-kernel" <linux-kernel@vger.kernel.org>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: [PATCH v5 20/21] selftests/resctrl: Fix checking for < 0 for unsigned values
Date: Sun,  7 Mar 2021 14:55:01 +0000	[thread overview]
Message-ID: <20210307145502.2916364-21-fenghua.yu@intel.com> (raw)
In-Reply-To: <20210307145502.2916364-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 de99d398ebfb..5a66e94cd4b4 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.30.1


  parent reply	other threads:[~2021-03-07 14:56 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 01/21] selftests/resctrl: Enable gcc checks to detect buffer overflows Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 02/21] selftests/resctrl: Fix compilation issues for global variables Fenghua Yu
2021-03-12 19:08   ` Babu Moger
2021-03-12 21:51     ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 03/21] selftests/resctrl: Fix compilation issues for other " Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 04/21] selftests/resctrl: Clean up resctrl features check Fenghua Yu
2021-03-12 19:09   ` Babu Moger
2021-03-12 22:01     ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 05/21] selftests/resctrl: Ensure sibling CPU is not same as original CPU Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 06/21] selftests/resctrl: Fix missing options "-n" and "-p" Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 07/21] selftests/resctrl: Rename CQM test as CMT test Fenghua Yu
2021-03-12 19:10   ` Babu Moger
2021-03-07 14:54 ` [PATCH v5 08/21] selftests/resctrl: Call kselftest APIs to log test results Fenghua Yu
2021-03-12 19:12   ` Babu Moger
2021-03-12 22:09     ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 09/21] selftests/resctrl: Share show_cache_info() by CAT and CMT tests Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 10/21] selftests/resctrl: Fix a printed message Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 11/21] selftests/resctrl: Add config dependencies Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 12/21] selftests/resctrl: Check for resctrl mount point only if resctrl FS is supported Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 13/21] selftests/resctrl: Use resctrl/info for feature detection Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 14/21] selftests/resctrl: Fix MBA/MBM results reporting format Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 15/21] selftests/resctrl: Don't hard code value of "no_of_bits" variable Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 16/21] selftests/resctrl: Modularize resctrl test suite main() function Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 17/21] selftests/resctrl: Skip the test if requested resctrl feature is not supported Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 18/21] selftests/resctrl: Fix unmount resctrl FS Fenghua Yu
2021-03-07 14:55 ` [PATCH v5 19/21] selftests/resctrl: Fix incorrect parsing of iMC counters Fenghua Yu
2021-03-07 14:55 ` Fenghua Yu [this message]
2021-03-07 14:55 ` [PATCH v5 21/21] selftests/resctrl: Create .gitignore to include resctrl_tests Fenghua Yu
2021-03-12 19:08 ` [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Babu Moger
2021-03-12 22:11   ` Fenghua Yu
2021-04-02 18:10     ` 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=20210307145502.2916364-21-fenghua.yu@intel.com \
    --to=fenghua.yu@intel.com \
    --cc=babu.moger@amd.com \
    --cc=dan.carpenter@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.