-----Original Message----- From: Fenghua Yu fenghua.yu@intel.com Sent: Sunday, March 7, 2021 8:55 AM To: Shuah Khan shuah@kernel.org; Tony Luck tony.luck@intel.com; Reinette Chatre reinette.chatre@intel.com; Moger, Babu 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 Subject: [PATCH v5 02/21] selftests/resctrl: Fix compilation issues for global variables
Reinette reported following compilation issue on Fedora 32, gcc version 10.1.1
/usr/bin/ld: cqm_test.o:<src_dir>/cqm_test.c:22: multiple definition of `cache_size'; cat_test.o:<src_dir>/cat_test.c:23: first defined here
The same issue is reported for long_mask, cbm_mask, count_of_bits etc variables as well. Compiler isn't happy because these variables are defined globally in two .c files namely cqm_test.c and cat_test.c and the compiler during compilation finds that the variable is already defined (multiple definition error).
Taking a closer look at the usage of these variables reveals that these variables are used only locally to functions such as cqm_resctrl_val()
%s/ locally to functions/locally in two functions
(defined in cqm_test.c) and cat_perf_miss_val() (defined in cat_test.c). These variables are not shared between those functions. So, there is no need for these variables to be global. Hence, fix this issue by making them static variables.
Reported-by: Reinette Chatre reinette.chatre@intel.com Signed-off-by: Fenghua Yu fenghua.yu@intel.com
Change Log: v5:
- Define long_mask, cbm_mask, count_of_bits etc as static variables (Shuah).
- Split this patch into patch 2 and 3 (Shuah).
tools/testing/selftests/resctrl/cat_test.c | 10 +++++----- tools/testing/selftests/resctrl/cqm_test.c | 10 +++++----- tools/testing/selftests/resctrl/resctrl.h | 2 +- tools/testing/selftests/resctrl/resctrlfs.c | 10 +++++----- 4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 5da43767b973..bdeeb5772592 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -17,10 +17,10 @@ #define MAX_DIFF_PERCENT 4 #define MAX_DIFF 1000000
-int count_of_bits; -char cbm_mask[256]; -unsigned long long_mask; -unsigned long cache_size; +static int count_of_bits; +static char cbm_mask[256]; +static unsigned long long_mask; +static unsigned long cache_size;
/*
- Change schemata. Write schemata to specified
@@ -136,7 +136,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) return -1;
/* 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/cqm_test.c b/tools/testing/selftests/resctrl/cqm_test.c index 5e7308ac63be..de33d1c0466e 100644 --- a/tools/testing/selftests/resctrl/cqm_test.c +++ b/tools/testing/selftests/resctrl/cqm_test.c @@ -16,10 +16,10 @@ #define MAX_DIFF 2000000 #define MAX_DIFF_PERCENT 15
-int count_of_bits; -char cbm_mask[256]; -unsigned long long_mask; -unsigned long cache_size; +static int count_of_bits; +static char cbm_mask[256]; +static unsigned long long_mask; +static unsigned long cache_size;
static int cqm_setup(int num, ...) { @@ -125,7 +125,7 @@ int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd) if (!validate_resctrl_feature_request("cqm")) return -1;
- 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 39bf59c6b9c5..959c71e39bdc 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -92,7 +92,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 19c0ec4045a4..2a16100c9c3f 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?
@@ -205,16 +203,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;
Can cbm_mask be NULL? I see it is statically allocated. Or should this be if (!(*cbm_mask))? Or did I miss something.
- sprintf(cbm_mask_path, "%s/%s/cbm_mask", CBM_MASK_PATH,
cache_type);
fp = fopen(cbm_mask_path, "r");
2.30.1