Date:   Tue, 20 Oct 2020 23:51:26 +0000
From:   Fenghua Yu <fenghua.yu@...el.com>
To:     "Shuah Khan" <shuah@...nel.org>,
        "Reinette Chatre" <reinette.chatre@...el.com>,
        "Tony Luck" <tony.luck@...el.com>,
        "Babu Moger" <babu.moger@....com>,
        "James Morse" <james.morse@....com>,
        "Borislav Petkov" <bp@...en8.de>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        "Ravi V Shankar" <ravi.v.shankar@...el.com>
Cc:     "linux-kselftest" <linux-kselftest@...r.kernel.org>,
        "linux-kernel" <linux-kernel@...r.kernel.org>,
        Fenghua Yu <fenghua.yu@...el.com>
Subject: [PATCH v3 21/21] selftests/resctrl: Don't use global variable for capacity bitmask (CBM)

Reinette reported following compilation issue on Fedora 32, gcc version
10.1.1

/usr/bin/ld: cmt_test.o:<src_dir>/cmt_test.c:19: multiple definition of
`cbm_mask'; cat_test.o:<src_dir>/cat_test.c:20: first defined here
/usr/bin/ld: resctrlfs.o:<src_dir>/resctrlfs.c:52: multiple definition of
`cbm_mask'; cat_test.o:<src_dir>/cat_test.c:20: first defined here

Compiler throws an error because cbm_mask is defined as a global
variable in three different .c files, namely resctrlfs.c, cat_test.c and
cmt_test.c.

cbm_mask is a global variable that is supposed to hold the capacity
bitmask (CBM) value read from resctrl/info directory. This is done by
get_cbm_mask() function which then sets the global variable.

To reduce the use of global variables (thereby improving readability and
maintainability) change get_cbm_mask() implementation such that it now
accepts a reference to a string. Upon success, get_cbm_mask() populates the
string with CBM value read from resctrl/info directory and returns 0 and on
failure, returns -1.

Please note that this change to get_cbm_mask() fixes the issue reported by
Reinette because cbm_mask isn't a global variable anymore.

Fixes: 78941183d1b15 ("selftests/resctrl: Add Cache QoS Monitoring (CQM) selftest")
Fixes: 790bf585b0eee ("selftests/resctrl: Add Cache Allocation Technology (CAT) selftest")
Reported-by: Reinette Chatre <reinette.chatre@...el.com>
Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
---
 tools/testing/selftests/resctrl/cat_test.c  |  5 ++---
 tools/testing/selftests/resctrl/cmt_test.c  |  5 ++---
 tools/testing/selftests/resctrl/resctrl.h   |  2 +-
 tools/testing/selftests/resctrl/resctrlfs.c | 10 +++++-----
 4 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index e518f8f7b4d8..60b31b85b484 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -17,8 +17,6 @@
 #define MAX_DIFF_PERCENT	4
 #define MAX_DIFF		1000000
 
-char cbm_mask[256];
-
 /*
  * Change schemata. Write schemata to specified
  * con_mon grp, mon_grp in resctrl FS.
@@ -120,6 +118,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
 {
 	unsigned long l_mask, l_mask_1, long_mask, cache_size;
 	int ret, pipefd[2], sibling_cpu_no, count_of_bits;
+	char cbm_mask[256];
 	char pipe_message;
 	pid_t bm_pid;
 
@@ -130,7 +129,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
 		return ret;
 
 	/* Get default cbm mask for L3/L2 cache */
-	ret = get_cbm_mask(cache_type);
+	ret = get_cbm_mask(cache_type, cbm_mask);
 	if (ret)
 		return ret;
 
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index f0cb441cca9f..0bee3e6480a3 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -16,8 +16,6 @@
 #define MAX_DIFF		2000000
 #define MAX_DIFF_PERCENT	15
 
-char cbm_mask[256];
-
 static int cmt_setup(int num, ...)
 {
 	struct resctrl_val_param *p;
@@ -112,6 +110,7 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
 {
 	int ret, mum_resctrlfs, count_of_bits;
 	unsigned long long_mask, cache_size;
+	char cbm_mask[256];
 
 	cache_size = 0;
 	mum_resctrlfs = 1;
@@ -120,7 +119,7 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
 	if (ret)
 		return ret;
 
-	ret = get_cbm_mask("L3");
+	ret = get_cbm_mask("L3", cbm_mask);
 	if (ret)
 		return ret;
 
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 65ca24bf3eac..2546435864d0 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -96,7 +96,7 @@ void tests_cleanup(void);
 void mbm_test_cleanup(void);
 int mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd);
 void mba_test_cleanup(void);
-int get_cbm_mask(char *cache_type);
+int get_cbm_mask(char *cache_type, char *cbm_mask);
 int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size);
 void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
 int cat_val(struct resctrl_val_param *param);
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 21281fd895b7..a07118e01590 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -49,8 +49,6 @@ static int find_resctrl_mount(char *buffer)
 	return -ENOENT;
 }
 
-char cbm_mask[256];
-
 /*
  * remount_resctrlfs - Remount resctrl FS at /sys/fs/resctrl
  * @mum_resctrlfs:	Should the resctrl FS be remounted?
@@ -208,16 +206,18 @@ int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size)
 /*
  * get_cbm_mask - Get cbm mask for given cache
  * @cache_type:	Cache level L2/L3
- *
- * Mask is stored in cbm_mask which is global variable.
+ * @cbm_mask:	cbm_mask returned as a string
  *
  * Return: = 0 on success, < 0 on failure.
  */
-int get_cbm_mask(char *cache_type)
+int get_cbm_mask(char *cache_type, char *cbm_mask)
 {
 	char cbm_mask_path[1024];
 	FILE *fp;
 
+	if (!cbm_mask)
+		return -1;
+
 	sprintf(cbm_mask_path, "%s/%s/cbm_mask", CBM_MASK_PATH, cache_type);
 
 	fp = fopen(cbm_mask_path, "r");
-- 
2.29.0

Powered by blists - more mailing lists