* [PATCH v5 01/21] selftests/resctrl: Enable gcc checks to detect buffer overflows
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
@ 2021-03-07 14:54 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 02/21] selftests/resctrl: Fix compilation issues for global variables Fenghua Yu
` (20 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu, David Binderman
David reported a buffer overflow error in the check_results() function of
the cmt unit test and he suggested enabling _FORTIFY_SOURCE gcc compiler
option to automatically detect any such errors.
Feature Test Macros man page describes_FORTIFY_SOURCE as below
"Defining this macro causes some lightweight checks to be performed to
detect some buffer overflow errors when employing various string and memory
manipulation functions (for example, memcpy, memset, stpcpy, strcpy,
strncpy, strcat, strncat, sprintf, snprintf, vsprintf, vsnprintf, gets, and
wide character variants thereof). For some functions, argument consistency
is checked; for example, a check is made that open has been supplied with a
mode argument when the specified flags include O_CREAT. Not all problems
are detected, just some common cases.
If _FORTIFY_SOURCE is set to 1, with compiler optimization level 1 (gcc
-O1) and above, checks that shouldn't change the behavior of conforming
programs are performed.
With _FORTIFY_SOURCE set to 2, some more checking is added, but some
conforming programs might fail.
Some of the checks can be performed at compile time (via macros logic
implemented in header files), and result in compiler warnings; other checks
take place at run time, and result in a run-time error if the check fails.
Use of this macro requires compiler support, available with gcc since
version 4.0."
Fix the buffer overflow error in the check_results() function of the cmt
unit test and enable _FORTIFY_SOURCE gcc check to catch any future buffer
overflow errors.
Reported-by: David Binderman <dcb314@hotmail.com>
Suggested-by: David Binderman <dcb314@hotmail.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Move from v4's patch 11 to patch 1 so the fix patch should be first
(Shuah).
tools/testing/selftests/resctrl/Makefile | 2 +-
tools/testing/selftests/resctrl/cqm_test.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile
index d585cc1948cc..6bcee2ec91a9 100644
--- a/tools/testing/selftests/resctrl/Makefile
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -1,5 +1,5 @@
CC = $(CROSS_COMPILE)gcc
-CFLAGS = -g -Wall
+CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2
SRCS=$(wildcard *.c)
OBJS=$(SRCS:.c=.o)
diff --git a/tools/testing/selftests/resctrl/cqm_test.c b/tools/testing/selftests/resctrl/cqm_test.c
index c8756152bd61..5e7308ac63be 100644
--- a/tools/testing/selftests/resctrl/cqm_test.c
+++ b/tools/testing/selftests/resctrl/cqm_test.c
@@ -86,7 +86,7 @@ static int check_results(struct resctrl_val_param *param, int no_of_bits)
return errno;
}
- while (fgets(temp, 1024, fp)) {
+ while (fgets(temp, sizeof(temp), fp)) {
char *token = strtok(temp, ":\t");
int fields = 0;
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 02/21] selftests/resctrl: Fix compilation issues for global variables
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 ` Fenghua Yu
2021-03-12 19:08 ` Babu Moger
2021-03-07 14:54 ` [PATCH v5 03/21] selftests/resctrl: Fix compilation issues for other " Fenghua Yu
` (19 subsequent siblings)
21 siblings, 1 reply; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
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()
(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;
+
sprintf(cbm_mask_path, "%s/%s/cbm_mask", CBM_MASK_PATH, cache_type);
fp = fopen(cbm_mask_path, "r");
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* RE: [PATCH v5 02/21] selftests/resctrl: Fix compilation issues for global variables
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
0 siblings, 1 reply; 32+ messages in thread
From: Babu Moger @ 2021-03-12 19:08 UTC (permalink / raw)
To: Fenghua Yu, Shuah Khan, Tony Luck, Reinette Chatre
Cc: linux-kselftest, linux-kernel
> -----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
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 02/21] selftests/resctrl: Fix compilation issues for global variables
2021-03-12 19:08 ` Babu Moger
@ 2021-03-12 21:51 ` Fenghua Yu
0 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-12 21:51 UTC (permalink / raw)
To: Babu Moger
Cc: Shuah Khan, Tony Luck, Reinette Chatre, linux-kselftest, linux-kernel
Hi, Babu,
On Fri, Mar 12, 2021 at 01:08:31PM -0600, Babu Moger wrote:
> > From: Fenghua Yu <fenghua.yu@intel.com>
> > 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
OK. Will change it.
> > -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.
This is a sanity checking. Although current callers do pass statically
allocated cbm_mask to the parameter, future callers may incorrectly pass
un-allocated cbm_mask to the parameter and may cause segmentation fault
without the sanity checking. To debug this kind of issue, the sanity
checking will be very helpful.
So I would keep this sanity checking.
Thanks.
-Fenghua
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v5 03/21] selftests/resctrl: Fix compilation issues for other global variables
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-07 14:54 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 04/21] selftests/resctrl: Clean up resctrl features check Fenghua Yu
` (18 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
Reinette reported following compilation issue on Fedora 32, gcc version
10.1.1
/usr/bin/ld: resctrl_tests.o:<src_dir>/resctrl.h:65: multiple definition
of `bm_pid'; cache.o:<src_dir>/resctrl.h:65: first defined here
Other variables are ppid, tests_run, llc_occup_path, is_amd. Compiler
isn't happy because these variables are defined globally in two .c files
but are not declared as extern.
To fix issues for the global variables, declare them as extern.
Chang Log:
- Split this patch from v4's patch 1 (Shuah).
Reported-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/resctrl.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 959c71e39bdc..12b77182cb44 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -62,11 +62,11 @@ struct resctrl_val_param {
int (*setup)(int num, ...);
};
-pid_t bm_pid, ppid;
-int tests_run;
+extern pid_t bm_pid, ppid;
+extern int tests_run;
-char llc_occup_path[1024];
-bool is_amd;
+extern char llc_occup_path[1024];
+extern bool is_amd;
bool check_resctrlfs_support(void);
int filter_dmesg(void);
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 04/21] selftests/resctrl: Clean up resctrl features check
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (2 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 03/21] selftests/resctrl: Fix compilation issues for other " Fenghua Yu
@ 2021-03-07 14:54 ` Fenghua Yu
2021-03-12 19:09 ` Babu Moger
2021-03-07 14:54 ` [PATCH v5 05/21] selftests/resctrl: Ensure sibling CPU is not same as original CPU Fenghua Yu
` (17 subsequent siblings)
21 siblings, 1 reply; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
Checking resctrl features call strcmp() to compare feature strings
(e.g. "mba", "cat" etc). The checkings are error prone and don't have
good coding style. Define the constant strings in macros and call
strncmp() to solve the potential issues.
Suggested-by: Shuah Khan <shuah@kernel.org>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Remove is_cat() etc functions and directly call strncmp() to check
the features (Shuah).
tools/testing/selftests/resctrl/cache.c | 8 +++----
tools/testing/selftests/resctrl/cat_test.c | 2 +-
tools/testing/selftests/resctrl/cqm_test.c | 2 +-
tools/testing/selftests/resctrl/fill_buf.c | 4 ++--
tools/testing/selftests/resctrl/mba_test.c | 2 +-
tools/testing/selftests/resctrl/mbm_test.c | 2 +-
tools/testing/selftests/resctrl/resctrl.h | 5 +++++
.../testing/selftests/resctrl/resctrl_tests.c | 12 +++++-----
tools/testing/selftests/resctrl/resctrl_val.c | 22 +++++++++----------
tools/testing/selftests/resctrl/resctrlfs.c | 17 +++++++-------
10 files changed, 41 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 38dbf4962e33..5922cc1b0386 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -182,7 +182,7 @@ int measure_cache_vals(struct resctrl_val_param *param, int bm_pid)
/*
* Measure cache miss from perf.
*/
- if (!strcmp(param->resctrl_val, "cat")) {
+ if (!strncmp(param->resctrl_val, CAT_STR, sizeof(CAT_STR))) {
ret = get_llc_perf(&llc_perf_miss);
if (ret < 0)
return ret;
@@ -192,7 +192,7 @@ int measure_cache_vals(struct resctrl_val_param *param, int bm_pid)
/*
* Measure llc occupancy from resctrl.
*/
- if (!strcmp(param->resctrl_val, "cqm")) {
+ if (!strncmp(param->resctrl_val, CQM_STR, sizeof(CQM_STR))) {
ret = get_llc_occu_resctrl(&llc_occu_resc);
if (ret < 0)
return ret;
@@ -234,7 +234,7 @@ int cat_val(struct resctrl_val_param *param)
if (ret)
return ret;
- if ((strcmp(resctrl_val, "cat") == 0)) {
+ if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) {
ret = initialize_llc_perf();
if (ret)
return ret;
@@ -242,7 +242,7 @@ int cat_val(struct resctrl_val_param *param)
/* Test runs until the callback setup() tells the test to stop. */
while (1) {
- if (strcmp(resctrl_val, "cat") == 0) {
+ if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) {
ret = param->setup(1, param);
if (ret) {
ret = 0;
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index bdeeb5772592..20823725daca 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -164,7 +164,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
return -1;
struct resctrl_val_param param = {
- .resctrl_val = "cat",
+ .resctrl_val = CAT_STR,
.cpu_no = cpu_no,
.mum_resctrlfs = 0,
.setup = cat_setup,
diff --git a/tools/testing/selftests/resctrl/cqm_test.c b/tools/testing/selftests/resctrl/cqm_test.c
index de33d1c0466e..271752e9ef5b 100644
--- a/tools/testing/selftests/resctrl/cqm_test.c
+++ b/tools/testing/selftests/resctrl/cqm_test.c
@@ -145,7 +145,7 @@ int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
}
struct resctrl_val_param param = {
- .resctrl_val = "cqm",
+ .resctrl_val = CQM_STR,
.ctrlgrp = "c1",
.mongrp = "m1",
.cpu_no = cpu_no,
diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
index 79c611c99a3d..51e5cf22632f 100644
--- a/tools/testing/selftests/resctrl/fill_buf.c
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -115,7 +115,7 @@ static int fill_cache_read(unsigned char *start_ptr, unsigned char *end_ptr,
while (1) {
ret = fill_one_span_read(start_ptr, end_ptr);
- if (!strcmp(resctrl_val, "cat"))
+ if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)))
break;
}
@@ -134,7 +134,7 @@ static int fill_cache_write(unsigned char *start_ptr, unsigned char *end_ptr,
{
while (1) {
fill_one_span_write(start_ptr, end_ptr);
- if (!strcmp(resctrl_val, "cat"))
+ if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)))
break;
}
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 7bf8eaa6204b..6449fbd96096 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -141,7 +141,7 @@ void mba_test_cleanup(void)
int mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd)
{
struct resctrl_val_param param = {
- .resctrl_val = "mba",
+ .resctrl_val = MBA_STR,
.ctrlgrp = "c1",
.mongrp = "m1",
.cpu_no = cpu_no,
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 4700f7453f81..ec6cfe01c9c2 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -114,7 +114,7 @@ void mbm_test_cleanup(void)
int mbm_bw_change(int span, int cpu_no, char *bw_report, char **benchmark_cmd)
{
struct resctrl_val_param param = {
- .resctrl_val = "mbm",
+ .resctrl_val = MBM_STR,
.ctrlgrp = "c1",
.mongrp = "m1",
.span = span,
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 12b77182cb44..36da6136af96 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -62,6 +62,11 @@ struct resctrl_val_param {
int (*setup)(int num, ...);
};
+#define MBM_STR "mbm"
+#define MBA_STR "mba"
+#define CQM_STR "cqm"
+#define CAT_STR "cat"
+
extern pid_t bm_pid, ppid;
extern int tests_run;
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index 425cc85ac883..4b109a59f72d 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -85,13 +85,13 @@ int main(int argc, char **argv)
cqm_test = false;
cat_test = false;
while (token) {
- if (!strcmp(token, "mbm")) {
+ if (!strncmp(token, MBM_STR, sizeof(MBM_STR))) {
mbm_test = true;
- } else if (!strcmp(token, "mba")) {
+ } else if (!strncmp(token, MBA_STR, sizeof(MBA_STR))) {
mba_test = true;
- } else if (!strcmp(token, "cqm")) {
+ } else if (!strncmp(token, CQM_STR, sizeof(CQM_STR))) {
cqm_test = true;
- } else if (!strcmp(token, "cat")) {
+ } else if (!strncmp(token, CAT_STR, sizeof(CAT_STR))) {
cat_test = true;
} else {
printf("invalid argument\n");
@@ -161,7 +161,7 @@ int main(int argc, char **argv)
if (!is_amd && mbm_test) {
printf("# Starting MBM BW change ...\n");
if (!has_ben)
- sprintf(benchmark_cmd[5], "%s", "mba");
+ sprintf(benchmark_cmd[5], "%s", MBA_STR);
res = mbm_bw_change(span, cpu_no, bw_report, benchmark_cmd);
printf("%sok MBM: bw change\n", res ? "not " : "");
mbm_test_cleanup();
@@ -181,7 +181,7 @@ int main(int argc, char **argv)
if (cqm_test) {
printf("# Starting CQM test ...\n");
if (!has_ben)
- sprintf(benchmark_cmd[5], "%s", "cqm");
+ sprintf(benchmark_cmd[5], "%s", CQM_STR);
res = cqm_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
printf("%sok CQM: test\n", res ? "not " : "");
cqm_test_cleanup();
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 520fea3606d1..aed71fd0713b 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -397,10 +397,10 @@ static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char *mongrp,
return;
}
- if (strcmp(resctrl_val, "mbm") == 0)
+ if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)))
set_mbm_path(ctrlgrp, mongrp, resource_id);
- if ((strcmp(resctrl_val, "mba") == 0)) {
+ if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
if (ctrlgrp)
sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH,
RESCTRL_PATH, ctrlgrp, resource_id);
@@ -524,7 +524,7 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
return;
}
- if (strcmp(resctrl_val, "cqm") == 0)
+ if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
set_cqm_path(ctrlgrp, mongrp, resource_id);
}
@@ -579,8 +579,8 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
if (strcmp(param->filename, "") == 0)
sprintf(param->filename, "stdio");
- if ((strcmp(resctrl_val, "mba")) == 0 ||
- (strcmp(resctrl_val, "mbm")) == 0) {
+ if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) ||
+ !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) {
ret = validate_bw_report_request(param->bw_report);
if (ret)
return ret;
@@ -674,15 +674,15 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
if (ret)
goto out;
- if ((strcmp(resctrl_val, "mbm") == 0) ||
- (strcmp(resctrl_val, "mba") == 0)) {
+ if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
+ !strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
ret = initialize_mem_bw_imc();
if (ret)
goto out;
initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp,
param->cpu_no, resctrl_val);
- } else if (strcmp(resctrl_val, "cqm") == 0)
+ } else if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
initialize_llc_occu_resctrl(param->ctrlgrp, param->mongrp,
param->cpu_no, resctrl_val);
@@ -710,8 +710,8 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
/* Test runs until the callback setup() tells the test to stop. */
while (1) {
- if ((strcmp(resctrl_val, "mbm") == 0) ||
- (strcmp(resctrl_val, "mba") == 0)) {
+ if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
+ !strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
ret = param->setup(1, param);
if (ret) {
ret = 0;
@@ -721,7 +721,7 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
ret = measure_vals(param, &bw_resc_start);
if (ret)
break;
- } else if (strcmp(resctrl_val, "cqm") == 0) {
+ } else if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR))) {
ret = param->setup(1, param);
if (ret) {
ret = 0;
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 2a16100c9c3f..4174e48e06d1 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -334,7 +334,7 @@ void run_benchmark(int signum, siginfo_t *info, void *ucontext)
operation = atoi(benchmark_cmd[4]);
sprintf(resctrl_val, "%s", benchmark_cmd[5]);
- if (strcmp(resctrl_val, "cqm") != 0)
+ if (strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
buffer_span = span * MB;
else
buffer_span = span;
@@ -459,8 +459,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
goto out;
/* Create mon grp and write pid into it for "mbm" and "cqm" test */
- if ((strcmp(resctrl_val, "cqm") == 0) ||
- (strcmp(resctrl_val, "mbm") == 0)) {
+ if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)) ||
+ !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) {
if (strlen(mongrp)) {
sprintf(monitorgroup_p, "%s/mon_groups", controlgroup);
sprintf(monitorgroup, "%s/%s", monitorgroup_p, mongrp);
@@ -505,9 +505,9 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
int resource_id, ret = 0;
FILE *fp;
- if ((strcmp(resctrl_val, "mba") != 0) &&
- (strcmp(resctrl_val, "cat") != 0) &&
- (strcmp(resctrl_val, "cqm") != 0))
+ if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) &&
+ strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) &&
+ strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
return -ENOENT;
if (!schemata) {
@@ -528,9 +528,10 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
else
sprintf(controlgroup, "%s/schemata", RESCTRL_PATH);
- if (!strcmp(resctrl_val, "cat") || !strcmp(resctrl_val, "cqm"))
+ if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) ||
+ !strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata);
- if (strcmp(resctrl_val, "mba") == 0)
+ if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)))
sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata);
fp = fopen(controlgroup, "w");
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* RE: [PATCH v5 04/21] selftests/resctrl: Clean up resctrl features check
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
0 siblings, 1 reply; 32+ messages in thread
From: Babu Moger @ 2021-03-12 19:09 UTC (permalink / raw)
To: Fenghua Yu, Shuah Khan, Tony Luck, Reinette Chatre
Cc: linux-kselftest, linux-kernel
> -----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 04/21] selftests/resctrl: Clean up resctrl features check
>
> Checking resctrl features call strcmp() to compare feature strings (e.g. "mba",
> "cat" etc). The checkings are error prone and don't have good coding style.
> Define the constant strings in macros and call
> strncmp() to solve the potential issues.
>
> Suggested-by: Shuah Khan <shuah@kernel.org>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
> Change Log:
> v5:
> - Remove is_cat() etc functions and directly call strncmp() to check
> the features (Shuah).
>
> tools/testing/selftests/resctrl/cache.c | 8 +++----
> tools/testing/selftests/resctrl/cat_test.c | 2 +-
> tools/testing/selftests/resctrl/cqm_test.c | 2 +-
> tools/testing/selftests/resctrl/fill_buf.c | 4 ++--
> tools/testing/selftests/resctrl/mba_test.c | 2 +-
> tools/testing/selftests/resctrl/mbm_test.c | 2 +-
> tools/testing/selftests/resctrl/resctrl.h | 5 +++++
> .../testing/selftests/resctrl/resctrl_tests.c | 12 +++++-----
> tools/testing/selftests/resctrl/resctrl_val.c | 22 +++++++++----------
> tools/testing/selftests/resctrl/resctrlfs.c | 17 +++++++-------
> 10 files changed, 41 insertions(+), 35 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/cache.c
> b/tools/testing/selftests/resctrl/cache.c
> index 38dbf4962e33..5922cc1b0386 100644
> --- a/tools/testing/selftests/resctrl/cache.c
> +++ b/tools/testing/selftests/resctrl/cache.c
> @@ -182,7 +182,7 @@ int measure_cache_vals(struct resctrl_val_param
> *param, int bm_pid)
> /*
> * Measure cache miss from perf.
> */
> - if (!strcmp(param->resctrl_val, "cat")) {
> + if (!strncmp(param->resctrl_val, CAT_STR, sizeof(CAT_STR))) {
> ret = get_llc_perf(&llc_perf_miss);
> if (ret < 0)
> return ret;
> @@ -192,7 +192,7 @@ int measure_cache_vals(struct resctrl_val_param
> *param, int bm_pid)
> /*
> * Measure llc occupancy from resctrl.
> */
> - if (!strcmp(param->resctrl_val, "cqm")) {
> + if (!strncmp(param->resctrl_val, CQM_STR, sizeof(CQM_STR))) {
> ret = get_llc_occu_resctrl(&llc_occu_resc);
> if (ret < 0)
> return ret;
> @@ -234,7 +234,7 @@ int cat_val(struct resctrl_val_param *param)
> if (ret)
> return ret;
>
> - if ((strcmp(resctrl_val, "cat") == 0)) {
> + if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) {
> ret = initialize_llc_perf();
> if (ret)
> return ret;
> @@ -242,7 +242,7 @@ int cat_val(struct resctrl_val_param *param)
>
> /* Test runs until the callback setup() tells the test to stop. */
> while (1) {
> - if (strcmp(resctrl_val, "cat") == 0) {
> + if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) {
> ret = param->setup(1, param);
> if (ret) {
> ret = 0;
> diff --git a/tools/testing/selftests/resctrl/cat_test.c
> b/tools/testing/selftests/resctrl/cat_test.c
> index bdeeb5772592..20823725daca 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -164,7 +164,7 @@ int cat_perf_miss_val(int cpu_no, int n, char
> *cache_type)
> return -1;
>
> struct resctrl_val_param param = {
> - .resctrl_val = "cat",
> + .resctrl_val = CAT_STR,
> .cpu_no = cpu_no,
> .mum_resctrlfs = 0,
> .setup = cat_setup,
> diff --git a/tools/testing/selftests/resctrl/cqm_test.c
> b/tools/testing/selftests/resctrl/cqm_test.c
> index de33d1c0466e..271752e9ef5b 100644
> --- a/tools/testing/selftests/resctrl/cqm_test.c
> +++ b/tools/testing/selftests/resctrl/cqm_test.c
> @@ -145,7 +145,7 @@ int cqm_resctrl_val(int cpu_no, int n, char
> **benchmark_cmd)
> }
>
> struct resctrl_val_param param = {
> - .resctrl_val = "cqm",
> + .resctrl_val = CQM_STR,
> .ctrlgrp = "c1",
> .mongrp = "m1",
> .cpu_no = cpu_no,
> diff --git a/tools/testing/selftests/resctrl/fill_buf.c
> b/tools/testing/selftests/resctrl/fill_buf.c
> index 79c611c99a3d..51e5cf22632f 100644
> --- a/tools/testing/selftests/resctrl/fill_buf.c
> +++ b/tools/testing/selftests/resctrl/fill_buf.c
> @@ -115,7 +115,7 @@ static int fill_cache_read(unsigned char *start_ptr,
> unsigned char *end_ptr,
>
> while (1) {
> ret = fill_one_span_read(start_ptr, end_ptr);
> - if (!strcmp(resctrl_val, "cat"))
> + if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)))
> break;
> }
>
> @@ -134,7 +134,7 @@ static int fill_cache_write(unsigned char *start_ptr,
> unsigned char *end_ptr, {
> while (1) {
> fill_one_span_write(start_ptr, end_ptr);
> - if (!strcmp(resctrl_val, "cat"))
> + if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)))
> break;
> }
>
> diff --git a/tools/testing/selftests/resctrl/mba_test.c
> b/tools/testing/selftests/resctrl/mba_test.c
> index 7bf8eaa6204b..6449fbd96096 100644
> --- a/tools/testing/selftests/resctrl/mba_test.c
> +++ b/tools/testing/selftests/resctrl/mba_test.c
> @@ -141,7 +141,7 @@ void mba_test_cleanup(void) int
> mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd)
> {
> struct resctrl_val_param param = {
> - .resctrl_val = "mba",
> + .resctrl_val = MBA_STR,
> .ctrlgrp = "c1",
> .mongrp = "m1",
> .cpu_no = cpu_no,
> diff --git a/tools/testing/selftests/resctrl/mbm_test.c
> b/tools/testing/selftests/resctrl/mbm_test.c
> index 4700f7453f81..ec6cfe01c9c2 100644
> --- a/tools/testing/selftests/resctrl/mbm_test.c
> +++ b/tools/testing/selftests/resctrl/mbm_test.c
> @@ -114,7 +114,7 @@ void mbm_test_cleanup(void) int mbm_bw_change(int
> span, int cpu_no, char *bw_report, char **benchmark_cmd) {
> struct resctrl_val_param param = {
> - .resctrl_val = "mbm",
> + .resctrl_val = MBM_STR,
> .ctrlgrp = "c1",
> .mongrp = "m1",
> .span = span,
> diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> index 12b77182cb44..36da6136af96 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -62,6 +62,11 @@ struct resctrl_val_param {
> int (*setup)(int num, ...);
> };
>
> +#define MBM_STR "mbm"
> +#define MBA_STR "mba"
> +#define CQM_STR "cqm"
> +#define CAT_STR "cat"
> +
> extern pid_t bm_pid, ppid;
> extern int tests_run;
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c
> b/tools/testing/selftests/resctrl/resctrl_tests.c
> index 425cc85ac883..4b109a59f72d 100644
> --- a/tools/testing/selftests/resctrl/resctrl_tests.c
> +++ b/tools/testing/selftests/resctrl/resctrl_tests.c
> @@ -85,13 +85,13 @@ int main(int argc, char **argv)
> cqm_test = false;
> cat_test = false;
> while (token) {
> - if (!strcmp(token, "mbm")) {
> + if (!strncmp(token, MBM_STR,
> sizeof(MBM_STR))) {
> mbm_test = true;
> - } else if (!strcmp(token, "mba")) {
> + } else if (!strncmp(token, MBA_STR,
> sizeof(MBA_STR))) {
> mba_test = true;
> - } else if (!strcmp(token, "cqm")) {
> + } else if (!strncmp(token, CQM_STR,
> sizeof(CQM_STR))) {
> cqm_test = true;
> - } else if (!strcmp(token, "cat")) {
> + } else if (!strncmp(token, CAT_STR,
> sizeof(CAT_STR))) {
> cat_test = true;
> } else {
> printf("invalid argument\n");
> @@ -161,7 +161,7 @@ int main(int argc, char **argv)
> if (!is_amd && mbm_test) {
> printf("# Starting MBM BW change ...\n");
> if (!has_ben)
> - sprintf(benchmark_cmd[5], "%s", "mba");
> + sprintf(benchmark_cmd[5], "%s", MBA_STR);
> res = mbm_bw_change(span, cpu_no, bw_report,
> benchmark_cmd);
> printf("%sok MBM: bw change\n", res ? "not " : "");
> mbm_test_cleanup();
> @@ -181,7 +181,7 @@ int main(int argc, char **argv)
> if (cqm_test) {
> printf("# Starting CQM test ...\n");
> if (!has_ben)
> - sprintf(benchmark_cmd[5], "%s", "cqm");
> + sprintf(benchmark_cmd[5], "%s", CQM_STR);
> res = cqm_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
> printf("%sok CQM: test\n", res ? "not " : "");
> cqm_test_cleanup();
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c
> b/tools/testing/selftests/resctrl/resctrl_val.c
> index 520fea3606d1..aed71fd0713b 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -397,10 +397,10 @@ static void initialize_mem_bw_resctrl(const char
> *ctrlgrp, const char *mongrp,
> return;
> }
>
> - if (strcmp(resctrl_val, "mbm") == 0)
> + if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)))
> set_mbm_path(ctrlgrp, mongrp, resource_id);
>
> - if ((strcmp(resctrl_val, "mba") == 0)) {
> + if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
> if (ctrlgrp)
> sprintf(mbm_total_path,
> CON_MBM_LOCAL_BYTES_PATH,
> RESCTRL_PATH, ctrlgrp, resource_id); @@ -
> 524,7 +524,7 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp,
> const char *mongrp,
> return;
> }
>
> - if (strcmp(resctrl_val, "cqm") == 0)
> + if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> set_cqm_path(ctrlgrp, mongrp, resource_id); }
>
> @@ -579,8 +579,8 @@ int resctrl_val(char **benchmark_cmd, struct
> resctrl_val_param *param)
> if (strcmp(param->filename, "") == 0)
> sprintf(param->filename, "stdio");
>
> - if ((strcmp(resctrl_val, "mba")) == 0 ||
> - (strcmp(resctrl_val, "mbm")) == 0) {
> + if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) ||
> + !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) {
> ret = validate_bw_report_request(param->bw_report);
> if (ret)
> return ret;
> @@ -674,15 +674,15 @@ int resctrl_val(char **benchmark_cmd, struct
> resctrl_val_param *param)
> if (ret)
> goto out;
>
> - if ((strcmp(resctrl_val, "mbm") == 0) ||
> - (strcmp(resctrl_val, "mba") == 0)) {
> + if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
> + !strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
> ret = initialize_mem_bw_imc();
> if (ret)
> goto out;
>
> initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp,
> param->cpu_no, resctrl_val);
> - } else if (strcmp(resctrl_val, "cqm") == 0)
> + } else if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> initialize_llc_occu_resctrl(param->ctrlgrp, param->mongrp,
> param->cpu_no, resctrl_val);
>
> @@ -710,8 +710,8 @@ int resctrl_val(char **benchmark_cmd, struct
> resctrl_val_param *param)
>
> /* Test runs until the callback setup() tells the test to stop. */
> while (1) {
> - if ((strcmp(resctrl_val, "mbm") == 0) ||
> - (strcmp(resctrl_val, "mba") == 0)) {
> + if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
> + !strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
> ret = param->setup(1, param);
> if (ret) {
> ret = 0;
> @@ -721,7 +721,7 @@ int resctrl_val(char **benchmark_cmd, struct
> resctrl_val_param *param)
> ret = measure_vals(param, &bw_resc_start);
> if (ret)
> break;
> - } else if (strcmp(resctrl_val, "cqm") == 0) {
> + } else if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR))) {
> ret = param->setup(1, param);
> if (ret) {
> ret = 0;
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
> b/tools/testing/selftests/resctrl/resctrlfs.c
> index 2a16100c9c3f..4174e48e06d1 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -334,7 +334,7 @@ void run_benchmark(int signum, siginfo_t *info, void
> *ucontext)
> operation = atoi(benchmark_cmd[4]);
> sprintf(resctrl_val, "%s", benchmark_cmd[5]);
>
> - if (strcmp(resctrl_val, "cqm") != 0)
> + if (strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> buffer_span = span * MB;
> else
> buffer_span = span;
> @@ -459,8 +459,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp,
> char *mongrp,
> goto out;
>
> /* Create mon grp and write pid into it for "mbm" and "cqm" test */
> - if ((strcmp(resctrl_val, "cqm") == 0) ||
> - (strcmp(resctrl_val, "mbm") == 0)) {
> + if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)) ||
> + !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) {
> if (strlen(mongrp)) {
> sprintf(monitorgroup_p, "%s/mon_groups",
> controlgroup);
> sprintf(monitorgroup, "%s/%s", monitorgroup_p,
> mongrp); @@ -505,9 +505,9 @@ int write_schemata(char *ctrlgrp, char
> *schemata, int cpu_no, char *resctrl_val)
> int resource_id, ret = 0;
> FILE *fp;
>
> - if ((strcmp(resctrl_val, "mba") != 0) &&
> - (strcmp(resctrl_val, "cat") != 0) &&
> - (strcmp(resctrl_val, "cqm") != 0))
> + if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) &&
> + strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) &&
> + strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> return -ENOENT;
>
> if (!schemata) {
> @@ -528,9 +528,10 @@ int write_schemata(char *ctrlgrp, char *schemata, int
> cpu_no, char *resctrl_val)
> else
> sprintf(controlgroup, "%s/schemata", RESCTRL_PATH);
>
> - if (!strcmp(resctrl_val, "cat") || !strcmp(resctrl_val, "cqm"))
> + if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) ||
> + !strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=',
> schemata);
> - if (strcmp(resctrl_val, "mba") == 0)
> + if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)))
> sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=',
> schemata);
>
> fp = fopen(controlgroup, "w");
> --
> 2.30.1
I see there are few other references as well. Like this.
1 cat_test.c cat_perf_miss_val 135 if
(!validate_resctrl_feature_request("cat"))
2 cqm_test.c cqm_resctrl_val 125 if
(!validate_resctrl_feature_request("cqm"))
3 mba_test.c mba_schemata_change 157 if
(!validate_resctrl_feature_request("mba"))
4 mbm_test.c mbm_bw_change 131 if
(!validate_resctrl_feature_request("mbm"))
Should you use CAT_STR and CQM_STR etc.. in here as well?
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 04/21] selftests/resctrl: Clean up resctrl features check
2021-03-12 19:09 ` Babu Moger
@ 2021-03-12 22:01 ` Fenghua Yu
0 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-12 22:01 UTC (permalink / raw)
To: Babu Moger
Cc: Shuah Khan, Tony Luck, Reinette Chatre, linux-kselftest, linux-kernel
Hi, Babu,
On Fri, Mar 12, 2021 at 01:09:50PM -0600, Babu Moger wrote:
> > - if (strcmp(resctrl_val, "mba") == 0)
> > + if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)))
> > sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=',
> > schemata);
> I see there are few other references as well. Like this.
>
> 1 cat_test.c cat_perf_miss_val 135 if
> (!validate_resctrl_feature_request("cat"))
>
> 2 cqm_test.c cqm_resctrl_val 125 if
> (!validate_resctrl_feature_request("cqm"))
>
> 3 mba_test.c mba_schemata_change 157 if
> (!validate_resctrl_feature_request("mba"))
>
> 4 mbm_test.c mbm_bw_change 131 if
> (!validate_resctrl_feature_request("mbm"))
>
> Should you use CAT_STR and CQM_STR etc.. in here as well?
Sure. I will fix this.
Thanks.
-Fenghua
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v5 05/21] selftests/resctrl: Ensure sibling CPU is not same as original CPU
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (3 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 04/21] selftests/resctrl: Clean up resctrl features check Fenghua Yu
@ 2021-03-07 14:54 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 06/21] selftests/resctrl: Fix missing options "-n" and "-p" Fenghua Yu
` (16 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
From: Reinette Chatre <reinette.chatre@intel.com>
The resctrl tests can accept a CPU on which the tests are run and use
default of CPU #1 if it is not provided. In the CAT test a "sibling CPU"
is determined that is from the same package where another thread will be
run.
The current algorithm with which a "sibling CPU" is determined does not
take the provided/default CPU into account and when that CPU is the
first CPU in a package then the "sibling CPU" will be selected to be the
same CPU since it starts by picking the first CPU from core_siblings_list.
Fix the "sibling CPU" selection by taking the provided/default CPU into
account and ensuring a sibling that is a different CPU is selected.
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Move from v4's patch 8 to this patch as the fix patch should be first
(Shuah).
tools/testing/selftests/resctrl/resctrlfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 4174e48e06d1..bc52076bee7f 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -268,7 +268,7 @@ int get_core_sibling(int cpu_no)
while (token) {
sibling_cpu_no = atoi(token);
/* Skipping core 0 as we don't want to run test on core 0 */
- if (sibling_cpu_no != 0)
+ if (sibling_cpu_no != 0 && sibling_cpu_no != cpu_no)
break;
token = strtok(NULL, "-,");
}
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 06/21] selftests/resctrl: Fix missing options "-n" and "-p"
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (4 preceding siblings ...)
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 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 07/21] selftests/resctrl: Rename CQM test as CMT test Fenghua Yu
` (15 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
resctrl test suite accepts command line arguments (like -b, -t, -n and -p)
as documented in the help. But passing -n and -p throws an invalid option
error. This happens because -n and -p are missing in the list of
characters that getopt() recognizes as valid arguments. Hence, they are
treated as invalid options.
Fix this by adding them to the list of characters that getopt() recognizes
as valid arguments. Please note that the main() function already has the
logic to deal with the values passed as part of these arguments and hence
no changes are needed there.
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Move from v4's patch 9 to this patch as the fix patch should be first
(Shuah).
tools/testing/selftests/resctrl/resctrl_tests.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index 4b109a59f72d..ac2269610aa9 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -73,7 +73,7 @@ int main(int argc, char **argv)
}
}
- while ((c = getopt(argc_new, argv, "ht:b:")) != -1) {
+ while ((c = getopt(argc_new, argv, "ht:b:n:p:")) != -1) {
char *token;
switch (c) {
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 07/21] selftests/resctrl: Rename CQM test as CMT test
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (5 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 06/21] selftests/resctrl: Fix missing options "-n" and "-p" Fenghua Yu
@ 2021-03-07 14:54 ` 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
` (14 subsequent siblings)
21 siblings, 1 reply; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
CMT (Cache Monitoring Technology) [1] is a H/W feature that reports cache
occupancy of a process. resctrl selftest suite has a unit test to test CMT
for LLC but the test is named as CQM (Cache Quality Monitoring).
Furthermore, the unit test source file is named as cqm_test.c and several
functions, variables, comments, preprocessors and statements widely use
"cqm" as either suffix or prefix. This rampant misusage of CQM for CMT
might confuse someone who is newly looking at resctrl selftests because
this feature is named CMT in the Intel Software Developer's Manual.
Hence, rename all the occurrences (unit test source file name, functions,
variables, comments and preprocessors) of cqm with cmt.
[1] Please see Intel SDM, Volume 3, chapter 17 and section 18 for more
information on CMT: https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html
Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/README | 4 +--
tools/testing/selftests/resctrl/cache.c | 4 +--
.../resctrl/{cqm_test.c => cmt_test.c} | 20 +++++++-------
tools/testing/selftests/resctrl/resctrl.h | 6 ++---
.../testing/selftests/resctrl/resctrl_tests.c | 26 +++++++++----------
tools/testing/selftests/resctrl/resctrl_val.c | 12 ++++-----
tools/testing/selftests/resctrl/resctrlfs.c | 10 +++----
7 files changed, 41 insertions(+), 41 deletions(-)
rename tools/testing/selftests/resctrl/{cqm_test.c => cmt_test.c} (89%)
diff --git a/tools/testing/selftests/resctrl/README b/tools/testing/selftests/resctrl/README
index 6e5a0ffa18e8..4b36b25b6ac0 100644
--- a/tools/testing/selftests/resctrl/README
+++ b/tools/testing/selftests/resctrl/README
@@ -46,8 +46,8 @@ ARGUMENTS
Parameter '-h' shows usage information.
usage: resctrl_tests [-h] [-b "benchmark_cmd [options]"] [-t test list] [-n no_of_bits]
- -b benchmark_cmd [options]: run specified benchmark for MBM, MBA and CQM default benchmark is builtin fill_buf
- -t test list: run tests specified in the test list, e.g. -t mbm, mba, cqm, cat
+ -b benchmark_cmd [options]: run specified benchmark for MBM, MBA and CMT default benchmark is builtin fill_buf
+ -t test list: run tests specified in the test list, e.g. -t mbm, mba, cmt, cat
-n no_of_bits: run cache tests using specified no of bits in cache bit mask
-p cpu_no: specify CPU number to run the test. 1 is default
-h: help
diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 5922cc1b0386..2aa1b5c7d9e1 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -111,7 +111,7 @@ static int get_llc_perf(unsigned long *llc_perf_miss)
/*
* Get LLC Occupancy as reported by RESCTRL FS
- * For CQM,
+ * For CMT,
* 1. If con_mon grp and mon grp given, then read from mon grp in
* con_mon grp
* 2. If only con_mon grp given, then read from con_mon grp
@@ -192,7 +192,7 @@ int measure_cache_vals(struct resctrl_val_param *param, int bm_pid)
/*
* Measure llc occupancy from resctrl.
*/
- if (!strncmp(param->resctrl_val, CQM_STR, sizeof(CQM_STR))) {
+ if (!strncmp(param->resctrl_val, CMT_STR, sizeof(CMT_STR))) {
ret = get_llc_occu_resctrl(&llc_occu_resc);
if (ret < 0)
return ret;
diff --git a/tools/testing/selftests/resctrl/cqm_test.c b/tools/testing/selftests/resctrl/cmt_test.c
similarity index 89%
rename from tools/testing/selftests/resctrl/cqm_test.c
rename to tools/testing/selftests/resctrl/cmt_test.c
index 271752e9ef5b..ca82db37c1f7 100644
--- a/tools/testing/selftests/resctrl/cqm_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * Cache Monitoring Technology (CQM) test
+ * Cache Monitoring Technology (CMT) test
*
* Copyright (C) 2018 Intel Corporation
*
@@ -11,7 +11,7 @@
#include "resctrl.h"
#include <unistd.h>
-#define RESULT_FILE_NAME "result_cqm"
+#define RESULT_FILE_NAME "result_cmt"
#define NUM_OF_RUNS 5
#define MAX_DIFF 2000000
#define MAX_DIFF_PERCENT 15
@@ -21,7 +21,7 @@ static char cbm_mask[256];
static unsigned long long_mask;
static unsigned long cache_size;
-static int cqm_setup(int num, ...)
+static int cmt_setup(int num, ...)
{
struct resctrl_val_param *p;
va_list param;
@@ -58,7 +58,7 @@ static void show_cache_info(unsigned long sum_llc_occu_resc, int no_of_bits,
else
res = false;
- printf("%sok CQM: diff within %d, %d\%%\n", res ? "" : "not",
+ printf("%sok CMT: diff within %d, %d\%%\n", res ? "" : "not",
MAX_DIFF, (int)MAX_DIFF_PERCENT);
printf("# diff: %ld\n", avg_diff);
@@ -106,12 +106,12 @@ static int check_results(struct resctrl_val_param *param, int no_of_bits)
return 0;
}
-void cqm_test_cleanup(void)
+void cmt_test_cleanup(void)
{
remove(RESULT_FILE_NAME);
}
-int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
+int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
{
int ret, mum_resctrlfs;
@@ -122,7 +122,7 @@ int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
if (ret)
return ret;
- if (!validate_resctrl_feature_request("cqm"))
+ if (!validate_resctrl_feature_request("cmt"))
return -1;
ret = get_cbm_mask("L3", cbm_mask);
@@ -145,7 +145,7 @@ int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
}
struct resctrl_val_param param = {
- .resctrl_val = CQM_STR,
+ .resctrl_val = CMT_STR,
.ctrlgrp = "c1",
.mongrp = "m1",
.cpu_no = cpu_no,
@@ -154,7 +154,7 @@ int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
.mask = ~(long_mask << n) & long_mask,
.span = cache_size * n / count_of_bits,
.num_of_runs = 0,
- .setup = cqm_setup,
+ .setup = cmt_setup,
};
if (strcmp(benchmark_cmd[0], "fill_buf") == 0)
@@ -170,7 +170,7 @@ int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
if (ret)
return ret;
- cqm_test_cleanup();
+ cmt_test_cleanup();
return 0;
}
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 36da6136af96..1a58767a0bd2 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -64,7 +64,7 @@ struct resctrl_val_param {
#define MBM_STR "mbm"
#define MBA_STR "mba"
-#define CQM_STR "cqm"
+#define CMT_STR "cmt"
#define CAT_STR "cat"
extern pid_t bm_pid, ppid;
@@ -103,9 +103,9 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
int cat_val(struct resctrl_val_param *param);
void cat_test_cleanup(void);
int cat_perf_miss_val(int cpu_no, int no_of_bits, char *cache_type);
-int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd);
+int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd);
unsigned int count_bits(unsigned long n);
-void cqm_test_cleanup(void);
+void cmt_test_cleanup(void);
int get_core_sibling(int cpu_no);
int measure_cache_vals(struct resctrl_val_param *param, int bm_pid);
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index ac2269610aa9..01f7aed7f54e 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -37,10 +37,10 @@ void detect_amd(void)
static void cmd_help(void)
{
printf("usage: resctrl_tests [-h] [-b \"benchmark_cmd [options]\"] [-t test list] [-n no_of_bits]\n");
- printf("\t-b benchmark_cmd [options]: run specified benchmark for MBM, MBA and CQM");
+ printf("\t-b benchmark_cmd [options]: run specified benchmark for MBM, MBA and CMT");
printf("\t default benchmark is builtin fill_buf\n");
printf("\t-t test list: run tests specified in the test list, ");
- printf("e.g. -t mbm, mba, cqm, cat\n");
+ printf("e.g. -t mbm, mba, cmt, cat\n");
printf("\t-n no_of_bits: run cache tests using specified no of bits in cache bit mask\n");
printf("\t-p cpu_no: specify CPU number to run the test. 1 is default\n");
printf("\t-h: help\n");
@@ -50,13 +50,13 @@ void tests_cleanup(void)
{
mbm_test_cleanup();
mba_test_cleanup();
- cqm_test_cleanup();
+ cmt_test_cleanup();
cat_test_cleanup();
}
int main(int argc, char **argv)
{
- bool has_ben = false, mbm_test = true, mba_test = true, cqm_test = true;
+ bool has_ben = false, mbm_test = true, mba_test = true, cmt_test = true;
int res, c, cpu_no = 1, span = 250, argc_new = argc, i, no_of_bits = 5;
char *benchmark_cmd[BENCHMARK_ARGS], bw_report[64], bm_type[64];
char benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE];
@@ -82,15 +82,15 @@ int main(int argc, char **argv)
mbm_test = false;
mba_test = false;
- cqm_test = false;
+ cmt_test = false;
cat_test = false;
while (token) {
if (!strncmp(token, MBM_STR, sizeof(MBM_STR))) {
mbm_test = true;
} else if (!strncmp(token, MBA_STR, sizeof(MBA_STR))) {
mba_test = true;
- } else if (!strncmp(token, CQM_STR, sizeof(CQM_STR))) {
- cqm_test = true;
+ } else if (!strncmp(token, CMT_STR, sizeof(CMT_STR))) {
+ cmt_test = true;
} else if (!strncmp(token, CAT_STR, sizeof(CAT_STR))) {
cat_test = true;
} else {
@@ -178,13 +178,13 @@ int main(int argc, char **argv)
tests_run++;
}
- if (cqm_test) {
- printf("# Starting CQM test ...\n");
+ if (cmt_test) {
+ printf("# Starting CMT test ...\n");
if (!has_ben)
- sprintf(benchmark_cmd[5], "%s", CQM_STR);
- res = cqm_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
- printf("%sok CQM: test\n", res ? "not " : "");
- cqm_test_cleanup();
+ sprintf(benchmark_cmd[5], "%s", "cmt");
+ res = cmt_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
+ printf("%sok CMT: test\n", res ? "not " : "");
+ cmt_test_cleanup();
tests_run++;
}
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index aed71fd0713b..17770095c98e 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -492,7 +492,7 @@ static int print_results_bw(char *filename, int bm_pid, float bw_imc,
return 0;
}
-static void set_cqm_path(const char *ctrlgrp, const char *mongrp, char sock_num)
+static void set_cmt_path(const char *ctrlgrp, const char *mongrp, char sock_num)
{
if (strlen(ctrlgrp) && strlen(mongrp))
sprintf(llc_occup_path, CON_MON_LCC_OCCUP_PATH, RESCTRL_PATH,
@@ -512,7 +512,7 @@ static void set_cqm_path(const char *ctrlgrp, const char *mongrp, char sock_num)
* @ctrlgrp: Name of the control monitor group (con_mon grp)
* @mongrp: Name of the monitor group (mon grp)
* @cpu_no: CPU number that the benchmark PID is binded to
- * @resctrl_val: Resctrl feature (Eg: cat, cqm.. etc)
+ * @resctrl_val: Resctrl feature (Eg: cat, cmt.. etc)
*/
static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
int cpu_no, char *resctrl_val)
@@ -524,8 +524,8 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
return;
}
- if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
- set_cqm_path(ctrlgrp, mongrp, resource_id);
+ if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
+ set_cmt_path(ctrlgrp, mongrp, resource_id);
}
static int
@@ -682,7 +682,7 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp,
param->cpu_no, resctrl_val);
- } else if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
+ } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
initialize_llc_occu_resctrl(param->ctrlgrp, param->mongrp,
param->cpu_no, resctrl_val);
@@ -721,7 +721,7 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
ret = measure_vals(param, &bw_resc_start);
if (ret)
break;
- } else if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR))) {
+ } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {
ret = param->setup(1, param);
if (ret) {
ret = 0;
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index bc52076bee7f..b47f4f150189 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -334,7 +334,7 @@ void run_benchmark(int signum, siginfo_t *info, void *ucontext)
operation = atoi(benchmark_cmd[4]);
sprintf(resctrl_val, "%s", benchmark_cmd[5]);
- if (strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
+ if (strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
buffer_span = span * MB;
else
buffer_span = span;
@@ -458,8 +458,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
if (ret)
goto out;
- /* Create mon grp and write pid into it for "mbm" and "cqm" test */
- if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)) ||
+ /* Create mon grp and write pid into it for "mbm" and "cmt" test */
+ if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)) ||
!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) {
if (strlen(mongrp)) {
sprintf(monitorgroup_p, "%s/mon_groups", controlgroup);
@@ -507,7 +507,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) &&
strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) &&
- strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
+ strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
return -ENOENT;
if (!schemata) {
@@ -529,7 +529,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
sprintf(controlgroup, "%s/schemata", RESCTRL_PATH);
if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) ||
- !strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
+ !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=', schemata);
if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)))
sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=', schemata);
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* RE: [PATCH v5 07/21] selftests/resctrl: Rename CQM test as CMT test
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
0 siblings, 0 replies; 32+ messages in thread
From: Babu Moger @ 2021-03-12 19:10 UTC (permalink / raw)
To: Fenghua Yu, Shuah Khan, Tony Luck, Reinette Chatre
Cc: linux-kselftest, linux-kernel
> -----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 07/21] selftests/resctrl: Rename CQM test as CMT test
>
> CMT (Cache Monitoring Technology) [1] is a H/W feature that reports cache
> occupancy of a process. resctrl selftest suite has a unit test to test CMT
> for LLC but the test is named as CQM (Cache Quality Monitoring).
> Furthermore, the unit test source file is named as cqm_test.c and several
> functions, variables, comments, preprocessors and statements widely use
> "cqm" as either suffix or prefix. This rampant misusage of CQM for CMT
> might confuse someone who is newly looking at resctrl selftests because
> this feature is named CMT in the Intel Software Developer's Manual.
>
> Hence, rename all the occurrences (unit test source file name, functions,
> variables, comments and preprocessors) of cqm with cmt.
>
> [1] Please see Intel SDM, Volume 3, chapter 17 and section 18 for more
> information on CMT:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsoftwar
> e.intel.com%2Fcontent%2Fwww%2Fus%2Fen%2Fdevelop%2Farticles%2Fintel-
> sdm.html&data=04%7C01%7Cbabu.moger%40amd.com%7C15bb0710f06a
> 4a413ee008d8e1790293%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0
> %7C637507257137108219%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAw
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata
> =f19HKVi8hIoN4CbNYheS4bV136HCbE3INJEum4wDJqo%3D&reserved=0
>
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
> tools/testing/selftests/resctrl/README | 4 +--
> tools/testing/selftests/resctrl/cache.c | 4 +--
> .../resctrl/{cqm_test.c => cmt_test.c} | 20 +++++++-------
> tools/testing/selftests/resctrl/resctrl.h | 6 ++---
> .../testing/selftests/resctrl/resctrl_tests.c | 26 +++++++++----------
> tools/testing/selftests/resctrl/resctrl_val.c | 12 ++++-----
> tools/testing/selftests/resctrl/resctrlfs.c | 10 +++----
> 7 files changed, 41 insertions(+), 41 deletions(-)
> rename tools/testing/selftests/resctrl/{cqm_test.c => cmt_test.c} (89%)
>
> diff --git a/tools/testing/selftests/resctrl/README
> b/tools/testing/selftests/resctrl/README
> index 6e5a0ffa18e8..4b36b25b6ac0 100644
> --- a/tools/testing/selftests/resctrl/README
> +++ b/tools/testing/selftests/resctrl/README
> @@ -46,8 +46,8 @@ ARGUMENTS
> Parameter '-h' shows usage information.
>
> usage: resctrl_tests [-h] [-b "benchmark_cmd [options]"] [-t test list] [-n
> no_of_bits]
> - -b benchmark_cmd [options]: run specified benchmark for MBM, MBA and
> CQM default benchmark is builtin fill_buf
> - -t test list: run tests specified in the test list, e.g. -t mbm, mba, cqm, cat
> + -b benchmark_cmd [options]: run specified benchmark for MBM, MBA and
> CMT default benchmark is builtin fill_buf
> + -t test list: run tests specified in the test list, e.g. -t mbm, mba, cmt, cat
> -n no_of_bits: run cache tests using specified no of bits in cache bit mask
> -p cpu_no: specify CPU number to run the test. 1 is default
> -h: help
> diff --git a/tools/testing/selftests/resctrl/cache.c
> b/tools/testing/selftests/resctrl/cache.c
> index 5922cc1b0386..2aa1b5c7d9e1 100644
> --- a/tools/testing/selftests/resctrl/cache.c
> +++ b/tools/testing/selftests/resctrl/cache.c
> @@ -111,7 +111,7 @@ static int get_llc_perf(unsigned long *llc_perf_miss)
>
> /*
> * Get LLC Occupancy as reported by RESCTRL FS
> - * For CQM,
> + * For CMT,
> * 1. If con_mon grp and mon grp given, then read from mon grp in
> * con_mon grp
> * 2. If only con_mon grp given, then read from con_mon grp
> @@ -192,7 +192,7 @@ int measure_cache_vals(struct resctrl_val_param
> *param, int bm_pid)
> /*
> * Measure llc occupancy from resctrl.
> */
> - if (!strncmp(param->resctrl_val, CQM_STR, sizeof(CQM_STR))) {
> + if (!strncmp(param->resctrl_val, CMT_STR, sizeof(CMT_STR))) {
> ret = get_llc_occu_resctrl(&llc_occu_resc);
> if (ret < 0)
> return ret;
> diff --git a/tools/testing/selftests/resctrl/cqm_test.c
> b/tools/testing/selftests/resctrl/cmt_test.c
> similarity index 89%
> rename from tools/testing/selftests/resctrl/cqm_test.c
> rename to tools/testing/selftests/resctrl/cmt_test.c
> index 271752e9ef5b..ca82db37c1f7 100644
> --- a/tools/testing/selftests/resctrl/cqm_test.c
> +++ b/tools/testing/selftests/resctrl/cmt_test.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * Cache Monitoring Technology (CQM) test
> + * Cache Monitoring Technology (CMT) test
> *
> * Copyright (C) 2018 Intel Corporation
> *
> @@ -11,7 +11,7 @@
> #include "resctrl.h"
> #include <unistd.h>
>
> -#define RESULT_FILE_NAME "result_cqm"
> +#define RESULT_FILE_NAME "result_cmt"
> #define NUM_OF_RUNS 5
> #define MAX_DIFF 2000000
> #define MAX_DIFF_PERCENT 15
> @@ -21,7 +21,7 @@ static char cbm_mask[256];
> static unsigned long long_mask;
> static unsigned long cache_size;
>
> -static int cqm_setup(int num, ...)
> +static int cmt_setup(int num, ...)
> {
> struct resctrl_val_param *p;
> va_list param;
> @@ -58,7 +58,7 @@ static void show_cache_info(unsigned long
> sum_llc_occu_resc, int no_of_bits,
> else
> res = false;
>
> - printf("%sok CQM: diff within %d, %d\%%\n", res ? "" : "not",
> + printf("%sok CMT: diff within %d, %d\%%\n", res ? "" : "not",
> MAX_DIFF, (int)MAX_DIFF_PERCENT);
>
> printf("# diff: %ld\n", avg_diff);
> @@ -106,12 +106,12 @@ static int check_results(struct resctrl_val_param
> *param, int no_of_bits)
> return 0;
> }
>
> -void cqm_test_cleanup(void)
> +void cmt_test_cleanup(void)
> {
> remove(RESULT_FILE_NAME);
> }
>
> -int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
> +int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
> {
> int ret, mum_resctrlfs;
>
> @@ -122,7 +122,7 @@ int cqm_resctrl_val(int cpu_no, int n, char
> **benchmark_cmd)
> if (ret)
> return ret;
>
> - if (!validate_resctrl_feature_request("cqm"))
> + if (!validate_resctrl_feature_request("cmt"))
> return -1;
>
> ret = get_cbm_mask("L3", cbm_mask);
> @@ -145,7 +145,7 @@ int cqm_resctrl_val(int cpu_no, int n, char
> **benchmark_cmd)
> }
>
> struct resctrl_val_param param = {
> - .resctrl_val = CQM_STR,
> + .resctrl_val = CMT_STR,
> .ctrlgrp = "c1",
> .mongrp = "m1",
> .cpu_no = cpu_no,
> @@ -154,7 +154,7 @@ int cqm_resctrl_val(int cpu_no, int n, char
> **benchmark_cmd)
> .mask = ~(long_mask << n) & long_mask,
> .span = cache_size * n / count_of_bits,
> .num_of_runs = 0,
> - .setup = cqm_setup,
> + .setup = cmt_setup,
> };
>
> if (strcmp(benchmark_cmd[0], "fill_buf") == 0)
> @@ -170,7 +170,7 @@ int cqm_resctrl_val(int cpu_no, int n, char
> **benchmark_cmd)
> if (ret)
> return ret;
>
> - cqm_test_cleanup();
> + cmt_test_cleanup();
>
> return 0;
> }
> diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> index 36da6136af96..1a58767a0bd2 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -64,7 +64,7 @@ struct resctrl_val_param {
>
> #define MBM_STR "mbm"
> #define MBA_STR "mba"
> -#define CQM_STR "cqm"
> +#define CMT_STR "cmt"
> #define CAT_STR "cat"
>
> extern pid_t bm_pid, ppid;
> @@ -103,9 +103,9 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
> int cat_val(struct resctrl_val_param *param);
> void cat_test_cleanup(void);
> int cat_perf_miss_val(int cpu_no, int no_of_bits, char *cache_type);
> -int cqm_resctrl_val(int cpu_no, int n, char **benchmark_cmd);
> +int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd);
> unsigned int count_bits(unsigned long n);
> -void cqm_test_cleanup(void);
> +void cmt_test_cleanup(void);
> int get_core_sibling(int cpu_no);
> int measure_cache_vals(struct resctrl_val_param *param, int bm_pid);
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c
> b/tools/testing/selftests/resctrl/resctrl_tests.c
> index ac2269610aa9..01f7aed7f54e 100644
> --- a/tools/testing/selftests/resctrl/resctrl_tests.c
> +++ b/tools/testing/selftests/resctrl/resctrl_tests.c
> @@ -37,10 +37,10 @@ void detect_amd(void)
> static void cmd_help(void)
> {
> printf("usage: resctrl_tests [-h] [-b \"benchmark_cmd [options]\"] [-t
> test list] [-n no_of_bits]\n");
> - printf("\t-b benchmark_cmd [options]: run specified benchmark for
> MBM, MBA and CQM");
> + printf("\t-b benchmark_cmd [options]: run specified benchmark for
> MBM, MBA and CMT");
> printf("\t default benchmark is builtin fill_buf\n");
> printf("\t-t test list: run tests specified in the test list, ");
> - printf("e.g. -t mbm, mba, cqm, cat\n");
> + printf("e.g. -t mbm, mba, cmt, cat\n");
> printf("\t-n no_of_bits: run cache tests using specified no of bits in
> cache bit mask\n");
> printf("\t-p cpu_no: specify CPU number to run the test. 1 is default\n");
> printf("\t-h: help\n");
> @@ -50,13 +50,13 @@ void tests_cleanup(void)
> {
> mbm_test_cleanup();
> mba_test_cleanup();
> - cqm_test_cleanup();
> + cmt_test_cleanup();
> cat_test_cleanup();
> }
>
> int main(int argc, char **argv)
> {
> - bool has_ben = false, mbm_test = true, mba_test = true, cqm_test =
> true;
> + bool has_ben = false, mbm_test = true, mba_test = true, cmt_test =
> true;
> int res, c, cpu_no = 1, span = 250, argc_new = argc, i, no_of_bits = 5;
> char *benchmark_cmd[BENCHMARK_ARGS], bw_report[64],
> bm_type[64];
> char
> benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE];
> @@ -82,15 +82,15 @@ int main(int argc, char **argv)
>
> mbm_test = false;
> mba_test = false;
> - cqm_test = false;
> + cmt_test = false;
> cat_test = false;
> while (token) {
> if (!strncmp(token, MBM_STR,
> sizeof(MBM_STR))) {
> mbm_test = true;
> } else if (!strncmp(token, MBA_STR,
> sizeof(MBA_STR))) {
> mba_test = true;
> - } else if (!strncmp(token, CQM_STR,
> sizeof(CQM_STR))) {
> - cqm_test = true;
> + } else if (!strncmp(token, CMT_STR,
> sizeof(CMT_STR))) {
> + cmt_test = true;
> } else if (!strncmp(token, CAT_STR,
> sizeof(CAT_STR))) {
> cat_test = true;
> } else {
> @@ -178,13 +178,13 @@ int main(int argc, char **argv)
> tests_run++;
> }
>
> - if (cqm_test) {
> - printf("# Starting CQM test ...\n");
> + if (cmt_test) {
> + printf("# Starting CMT test ...\n");
> if (!has_ben)
> - sprintf(benchmark_cmd[5], "%s", CQM_STR);
> - res = cqm_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
> - printf("%sok CQM: test\n", res ? "not " : "");
> - cqm_test_cleanup();
> + sprintf(benchmark_cmd[5], "%s", "cmt");
Shouldn't this be sprintf(benchmark_cmd[5], "%s", CMT_STR); ?
> + res = cmt_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
> + printf("%sok CMT: test\n", res ? "not " : "");
> + cmt_test_cleanup();
> tests_run++;
> }
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c
> b/tools/testing/selftests/resctrl/resctrl_val.c
> index aed71fd0713b..17770095c98e 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -492,7 +492,7 @@ static int print_results_bw(char *filename, int bm_pid,
> float bw_imc,
> return 0;
> }
>
> -static void set_cqm_path(const char *ctrlgrp, const char *mongrp, char
> sock_num)
> +static void set_cmt_path(const char *ctrlgrp, const char *mongrp, char
> sock_num)
> {
> if (strlen(ctrlgrp) && strlen(mongrp))
> sprintf(llc_occup_path, CON_MON_LCC_OCCUP_PATH,
> RESCTRL_PATH,
> @@ -512,7 +512,7 @@ static void set_cqm_path(const char *ctrlgrp, const char
> *mongrp, char sock_num)
> * @ctrlgrp: Name of the control monitor group (con_mon
> grp)
> * @mongrp: Name of the monitor group (mon grp)
> * @cpu_no: CPU number that the benchmark PID is binded
> to
> - * @resctrl_val: Resctrl feature (Eg: cat, cqm.. etc)
> + * @resctrl_val: Resctrl feature (Eg: cat, cmt.. etc)
> */
> static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
> int cpu_no, char *resctrl_val)
> @@ -524,8 +524,8 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp,
> const char *mongrp,
> return;
> }
>
> - if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> - set_cqm_path(ctrlgrp, mongrp, resource_id);
> + if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
> + set_cmt_path(ctrlgrp, mongrp, resource_id);
> }
>
> static int
> @@ -682,7 +682,7 @@ int resctrl_val(char **benchmark_cmd, struct
> resctrl_val_param *param)
>
> initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp,
> param->cpu_no, resctrl_val);
> - } else if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> + } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
> initialize_llc_occu_resctrl(param->ctrlgrp, param->mongrp,
> param->cpu_no, resctrl_val);
>
> @@ -721,7 +721,7 @@ int resctrl_val(char **benchmark_cmd, struct
> resctrl_val_param *param)
> ret = measure_vals(param, &bw_resc_start);
> if (ret)
> break;
> - } else if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR))) {
> + } else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {
> ret = param->setup(1, param);
> if (ret) {
> ret = 0;
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
> b/tools/testing/selftests/resctrl/resctrlfs.c
> index bc52076bee7f..b47f4f150189 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -334,7 +334,7 @@ void run_benchmark(int signum, siginfo_t *info, void
> *ucontext)
> operation = atoi(benchmark_cmd[4]);
> sprintf(resctrl_val, "%s", benchmark_cmd[5]);
>
> - if (strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> + if (strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
> buffer_span = span * MB;
> else
> buffer_span = span;
> @@ -458,8 +458,8 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp,
> char *mongrp,
> if (ret)
> goto out;
>
> - /* Create mon grp and write pid into it for "mbm" and "cqm" test */
> - if (!strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)) ||
> + /* Create mon grp and write pid into it for "mbm" and "cmt" test */
> + if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)) ||
> !strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) {
> if (strlen(mongrp)) {
> sprintf(monitorgroup_p, "%s/mon_groups",
> controlgroup);
> @@ -507,7 +507,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int
> cpu_no, char *resctrl_val)
>
> if (strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)) &&
> strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) &&
> - strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> + strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
> return -ENOENT;
>
> if (!schemata) {
> @@ -529,7 +529,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int
> cpu_no, char *resctrl_val)
> sprintf(controlgroup, "%s/schemata", RESCTRL_PATH);
>
> if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)) ||
> - !strncmp(resctrl_val, CQM_STR, sizeof(CQM_STR)))
> + !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
> sprintf(schema, "%s%d%c%s", "L3:", resource_id, '=',
> schemata);
> if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR)))
> sprintf(schema, "%s%d%c%s", "MB:", resource_id, '=',
> schemata);
> --
> 2.30.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v5 08/21] selftests/resctrl: Call kselftest APIs to log test results
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (6 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 07/21] selftests/resctrl: Rename CQM test as CMT test Fenghua Yu
@ 2021-03-07 14:54 ` Fenghua Yu
2021-03-12 19:12 ` Babu Moger
2021-03-07 14:54 ` [PATCH v5 09/21] selftests/resctrl: Share show_cache_info() by CAT and CMT tests Fenghua Yu
` (13 subsequent siblings)
21 siblings, 1 reply; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
Call kselftest APIs instead of using printf() to log test results
for cleaner code and better future extension.
Suggested-by: Shuah Khan <shuah@kernel.org>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Add this patch (Shuah)
tools/testing/selftests/resctrl/cat_test.c | 37 +++++++--------
tools/testing/selftests/resctrl/cmt_test.c | 42 ++++++++---------
tools/testing/selftests/resctrl/mba_test.c | 24 +++++-----
tools/testing/selftests/resctrl/mbm_test.c | 28 ++++++------
tools/testing/selftests/resctrl/resctrl.h | 2 +-
.../testing/selftests/resctrl/resctrl_tests.c | 40 +++++++++--------
tools/testing/selftests/resctrl/resctrl_val.c | 4 +-
tools/testing/selftests/resctrl/resctrlfs.c | 45 +++++++------------
8 files changed, 105 insertions(+), 117 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 20823725daca..0deb38ed971b 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -52,25 +52,28 @@ static int cat_setup(int num, ...)
return ret;
}
-static void show_cache_info(unsigned long sum_llc_perf_miss, int no_of_bits,
- unsigned long span)
+static int show_cache_info(unsigned long sum_llc_perf_miss, int no_of_bits,
+ unsigned long span)
{
unsigned long allocated_cache_lines = span / 64;
unsigned long avg_llc_perf_miss = 0;
float diff_percent;
+ int ret;
avg_llc_perf_miss = sum_llc_perf_miss / (NUM_OF_RUNS - 1);
diff_percent = ((float)allocated_cache_lines - avg_llc_perf_miss) /
allocated_cache_lines * 100;
- printf("%sok CAT: cache miss rate within %d%%\n",
- !is_amd && abs((int)diff_percent) > MAX_DIFF_PERCENT ?
- "not " : "", MAX_DIFF_PERCENT);
- tests_run++;
- printf("# Percent diff=%d\n", abs((int)diff_percent));
- printf("# Number of bits: %d\n", no_of_bits);
- printf("# Avg_llc_perf_miss: %lu\n", avg_llc_perf_miss);
- printf("# Allocated cache lines: %lu\n", allocated_cache_lines);
+ ret = !is_amd && abs((int)diff_percent) > MAX_DIFF_PERCENT;
+ ksft_print_msg("cache miss rate %swithin %d%%\n",
+ ret ? "not " : "", MAX_DIFF_PERCENT);
+
+ ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent));
+ ksft_print_msg("Number of bits: %d\n", no_of_bits);
+ ksft_print_msg("Avg_llc_perf_miss: %lu\n", avg_llc_perf_miss);
+ ksft_print_msg("Allocated cache lines: %lu\n", allocated_cache_lines);
+
+ return ret;
}
static int check_results(struct resctrl_val_param *param)
@@ -80,7 +83,7 @@ static int check_results(struct resctrl_val_param *param)
int runs = 0, no_of_bits = 0;
FILE *fp;
- printf("# Checking for pass/fail\n");
+ ksft_print_msg("Checking for pass/fail\n");
fp = fopen(param->filename, "r");
if (!fp) {
perror("# Cannot open file");
@@ -108,9 +111,7 @@ static int check_results(struct resctrl_val_param *param)
fclose(fp);
no_of_bits = count_bits(param->mask);
- show_cache_info(sum_llc_perf_miss, no_of_bits, param->span);
-
- return 0;
+ return show_cache_info(sum_llc_perf_miss, no_of_bits, param->span);
}
void cat_test_cleanup(void)
@@ -146,15 +147,15 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
ret = get_cache_size(cpu_no, cache_type, &cache_size);
if (ret)
return ret;
- printf("cache size :%lu\n", cache_size);
+ ksft_print_msg("cache size :%lu\n", cache_size);
/* Get max number of bits from default-cabm mask */
count_of_bits = count_bits(long_mask);
if (n < 1 || n > count_of_bits - 1) {
- printf("Invalid input value for no_of_bits n!\n");
- printf("Please Enter value in range 1 to %d\n",
- count_of_bits - 1);
+ ksft_print_msg("Invalid input value for no_of_bits n!\n");
+ ksft_print_msg("Please Enter value in range 1 to %d\n",
+ count_of_bits - 1);
return -1;
}
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index ca82db37c1f7..e5af19335115 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -39,36 +39,33 @@ static int cmt_setup(int num, ...)
return 0;
}
-static void show_cache_info(unsigned long sum_llc_occu_resc, int no_of_bits,
- unsigned long span)
+static int show_cache_info(unsigned long sum_llc_occu_resc, int no_of_bits,
+ unsigned long span)
{
unsigned long avg_llc_occu_resc = 0;
float diff_percent;
long avg_diff = 0;
- bool res;
+ int ret;
avg_llc_occu_resc = sum_llc_occu_resc / (NUM_OF_RUNS - 1);
avg_diff = (long)abs(span - avg_llc_occu_resc);
diff_percent = (((float)span - avg_llc_occu_resc) / span) * 100;
- if ((abs((int)diff_percent) <= MAX_DIFF_PERCENT) ||
- (abs(avg_diff) <= MAX_DIFF))
- res = true;
- else
- res = false;
+ ret = (abs((int)diff_percent) > MAX_DIFF_PERCENT) &&
+ (abs(avg_diff) > MAX_DIFF);
- printf("%sok CMT: diff within %d, %d\%%\n", res ? "" : "not",
- MAX_DIFF, (int)MAX_DIFF_PERCENT);
+ ksft_print_msg("%scache miss diff within %d, %d\%%\n",
+ ret ? "fail: " : "not", MAX_DIFF, (int)MAX_DIFF_PERCENT);
- printf("# diff: %ld\n", avg_diff);
- printf("# percent diff=%d\n", abs((int)diff_percent));
- printf("# Results are displayed in (Bytes)\n");
- printf("# Number of bits: %d\n", no_of_bits);
- printf("# Avg_llc_occu_resc: %lu\n", avg_llc_occu_resc);
- printf("# llc_occu_exp (span): %lu\n", span);
+ ksft_print_msg("diff: %ld\n", avg_diff);
+ ksft_print_msg("percent diff=%d\n", abs((int)diff_percent));
+ ksft_print_msg("Results are displayed in (Bytes)\n");
+ ksft_print_msg("Number of bits: %d\n", no_of_bits);
+ ksft_print_msg("Avg_llc_occu_resc: %lu\n", avg_llc_occu_resc);
+ ksft_print_msg("llc_occu_exp (span): %lu\n", span);
- tests_run++;
+ return ret;
}
static int check_results(struct resctrl_val_param *param, int no_of_bits)
@@ -78,7 +75,7 @@ static int check_results(struct resctrl_val_param *param, int no_of_bits)
int runs = 0;
FILE *fp;
- printf("# checking for pass/fail\n");
+ ksft_print_msg("checking for pass/fail\n");
fp = fopen(param->filename, "r");
if (!fp) {
perror("# Error in opening file\n");
@@ -101,9 +98,8 @@ static int check_results(struct resctrl_val_param *param, int no_of_bits)
runs++;
}
fclose(fp);
- show_cache_info(sum_llc_occu_resc, no_of_bits, param->span);
- return 0;
+ return show_cache_info(sum_llc_occu_resc, no_of_bits, param->span);
}
void cmt_test_cleanup(void)
@@ -134,13 +130,13 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
ret = get_cache_size(cpu_no, "L3", &cache_size);
if (ret)
return ret;
- printf("cache size :%lu\n", cache_size);
+ ksft_print_msg("cache size :%lu\n", cache_size);
count_of_bits = count_bits(long_mask);
if (n < 1 || n > count_of_bits) {
- printf("Invalid input value for numbr_of_bits n!\n");
- printf("Please Enter value in range 1 to %d\n", count_of_bits);
+ ksft_print_msg("Invalid input value for numbr_of_bits n!\n");
+ ksft_print_msg("Please Enter value in range 1 to %d\n", count_of_bits);
return -1;
}
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 6449fbd96096..3a226effe80c 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -56,7 +56,7 @@ static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
int allocation, runs;
bool failed = false;
- printf("# Results are displayed in (MB)\n");
+ ksft_print_msg("Results are displayed in (MB)\n");
/* Memory bandwidth from 100% down to 10% */
for (allocation = 0; allocation < ALLOCATION_MAX / ALLOCATION_STEP;
allocation++) {
@@ -78,21 +78,21 @@ static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
avg_bw_resc = sum_bw_resc / (NUM_OF_RUNS - 1);
avg_diff = labs((long)(avg_bw_resc - avg_bw_imc));
- printf("%sok MBA schemata percentage %u smaller than %d %%\n",
- avg_diff > MAX_DIFF ? "not " : "",
- ALLOCATION_MAX - ALLOCATION_STEP * allocation,
- MAX_DIFF);
- tests_run++;
- printf("# avg_diff: %lu\n", avg_diff);
- printf("# avg_bw_imc: %lu\n", avg_bw_imc);
- printf("# avg_bw_resc: %lu\n", avg_bw_resc);
+ ksft_print_msg("%s MBA schemata percentage %u smaller than %d %%\n",
+ avg_diff > MAX_DIFF ? "fail:" : "pass:",
+ ALLOCATION_MAX - ALLOCATION_STEP * allocation,
+ MAX_DIFF);
+ ksft_print_msg("avg_diff: %lu\n", avg_diff);
+ ksft_print_msg("avg_bw_imc: %lu\n", avg_bw_imc);
+ ksft_print_msg("avg_bw_resc: %lu\n", avg_bw_resc);
if (avg_diff > MAX_DIFF)
failed = true;
}
- printf("%sok schemata change using MBA%s\n", failed ? "not " : "",
- failed ? " # at least one test failed" : "");
- tests_run++;
+ ksft_print_msg("%s schemata change using MBA\n",
+ failed ? "fail:" : "pass:");
+ if (failed)
+ ksft_print_msg("at least one test failed");
}
static int check_results(void)
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index ec6cfe01c9c2..2b4f26013d84 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -14,13 +14,13 @@
#define MAX_DIFF 300
#define NUM_OF_RUNS 5
-static void
+static int
show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span)
{
unsigned long avg_bw_imc = 0, avg_bw_resc = 0;
unsigned long sum_bw_imc = 0, sum_bw_resc = 0;
long avg_diff = 0;
- int runs;
+ int runs, ret;
/*
* Discard the first value which is inaccurate due to monitoring setup
@@ -35,13 +35,15 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span)
avg_bw_resc = sum_bw_resc / 4;
avg_diff = avg_bw_resc - avg_bw_imc;
- printf("%sok MBM: diff within %d%%\n",
- labs(avg_diff) > MAX_DIFF ? "not " : "", MAX_DIFF);
- tests_run++;
- printf("# avg_diff: %lu\n", labs(avg_diff));
- printf("# Span (MB): %d\n", span);
- printf("# avg_bw_imc: %lu\n", avg_bw_imc);
- printf("# avg_bw_resc: %lu\n", avg_bw_resc);
+ ret = labs(avg_diff) > MAX_DIFF;
+ ksft_print_msg("%s MBM: diff within %d%%\n",
+ ret ? "fail:" : "pass:", MAX_DIFF);
+ ksft_print_msg("avg_diff: %lu\n", labs(avg_diff));
+ ksft_print_msg("Span (MB): %d\n", span);
+ ksft_print_msg("avg_bw_imc: %lu\n", avg_bw_imc);
+ ksft_print_msg("avg_bw_resc: %lu\n", avg_bw_resc);
+
+ return ret;
}
static int check_results(int span)
@@ -49,10 +51,10 @@ static int check_results(int span)
unsigned long bw_imc[NUM_OF_RUNS], bw_resc[NUM_OF_RUNS];
char temp[1024], *token_array[8];
char output[] = RESULT_FILE_NAME;
- int runs;
+ int runs, ret;
FILE *fp;
- printf("# Checking for pass/fail\n");
+ ksft_print_msg("Checking for pass/fail\n");
fp = fopen(output, "r");
if (!fp) {
@@ -76,11 +78,11 @@ static int check_results(int span)
runs++;
}
- show_bw_info(bw_imc, bw_resc, span);
+ ret = show_bw_info(bw_imc, bw_resc, span);
fclose(fp);
- return 0;
+ return ret;
}
static int mbm_setup(int num, ...)
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 1a58767a0bd2..ebf88217f9de 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -23,6 +23,7 @@
#include <sys/eventfd.h>
#include <asm/unistd.h>
#include <linux/perf_event.h>
+#include "../kselftest.h"
#define MB (1024 * 1024)
#define RESCTRL_PATH "/sys/fs/resctrl"
@@ -68,7 +69,6 @@ struct resctrl_val_param {
#define CAT_STR "cat"
extern pid_t bm_pid, ppid;
-extern int tests_run;
extern char llc_occup_path[1024];
extern bool is_amd;
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index 01f7aed7f54e..ebc24992cc2c 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -60,7 +60,7 @@ int main(int argc, char **argv)
int res, c, cpu_no = 1, span = 250, argc_new = argc, i, no_of_bits = 5;
char *benchmark_cmd[BENCHMARK_ARGS], bw_report[64], bm_type[64];
char benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE];
- int ben_ind, ben_count;
+ int ben_ind, ben_count, tests = 0;
bool cat_test = true;
for (i = 0; i < argc; i++) {
@@ -87,12 +87,16 @@ int main(int argc, char **argv)
while (token) {
if (!strncmp(token, MBM_STR, sizeof(MBM_STR))) {
mbm_test = true;
+ tests++;
} else if (!strncmp(token, MBA_STR, sizeof(MBA_STR))) {
mba_test = true;
+ tests++;
} else if (!strncmp(token, CMT_STR, sizeof(CMT_STR))) {
cmt_test = true;
+ tests++;
} else if (!strncmp(token, CAT_STR, sizeof(CAT_STR))) {
cat_test = true;
+ tests++;
} else {
printf("invalid argument\n");
@@ -118,7 +122,7 @@ int main(int argc, char **argv)
}
}
- printf("TAP version 13\n");
+ ksft_print_header();
/*
* Typically we need root privileges, because:
@@ -126,7 +130,7 @@ int main(int argc, char **argv)
* 2. We execute perf commands
*/
if (geteuid() != 0)
- printf("# WARNING: not running as root, tests may fail.\n");
+ return ksft_exit_fail_msg("not running as root, abort testing.\n");
/* Detect AMD vendor */
detect_amd();
@@ -155,48 +159,46 @@ int main(int argc, char **argv)
sprintf(bw_report, "reads");
sprintf(bm_type, "fill_buf");
- check_resctrlfs_support();
+ if (!check_resctrlfs_support())
+ return ksft_exit_fail_msg("resctrl FS does not exist\n");
+
filter_dmesg();
+ ksft_set_plan(tests ? : 4);
+
if (!is_amd && mbm_test) {
- printf("# Starting MBM BW change ...\n");
+ ksft_print_msg("Starting MBM BW change ...\n");
if (!has_ben)
sprintf(benchmark_cmd[5], "%s", MBA_STR);
res = mbm_bw_change(span, cpu_no, bw_report, benchmark_cmd);
- printf("%sok MBM: bw change\n", res ? "not " : "");
+ ksft_test_result(!res, "MBM: bw change\n");
mbm_test_cleanup();
- tests_run++;
}
if (!is_amd && mba_test) {
- printf("# Starting MBA Schemata change ...\n");
+ ksft_print_msg("Starting MBA Schemata change ...\n");
if (!has_ben)
sprintf(benchmark_cmd[1], "%d", span);
res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd);
- printf("%sok MBA: schemata change\n", res ? "not " : "");
+ ksft_test_result(!res, "MBA: schemata change\n");
mba_test_cleanup();
- tests_run++;
}
if (cmt_test) {
- printf("# Starting CMT test ...\n");
+ ksft_print_msg("Starting CMT test ...\n");
if (!has_ben)
sprintf(benchmark_cmd[5], "%s", "cmt");
res = cmt_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
- printf("%sok CMT: test\n", res ? "not " : "");
+ ksft_test_result(!res, "CMT: test\n");
cmt_test_cleanup();
- tests_run++;
}
if (cat_test) {
- printf("# Starting CAT test ...\n");
+ ksft_print_msg("Starting CAT test ...\n");
res = cat_perf_miss_val(cpu_no, no_of_bits, "L3");
- printf("%sok CAT: test\n", res ? "not " : "");
- tests_run++;
+ ksft_test_result(!res, "CAT: test\n");
cat_test_cleanup();
}
- printf("1..%d\n", tests_run);
-
- return 0;
+ return ksft_exit_pass();
}
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 17770095c98e..48bcd5fd7d79 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -449,7 +449,7 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr)
kill(bm_pid, SIGKILL);
umount_resctrlfs();
tests_cleanup();
- printf("Ending\n\n");
+ ksft_print_msg("Ending\n\n");
exit(EXIT_SUCCESS);
}
@@ -645,7 +645,7 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
PARENT_EXIT("Child is done");
}
- printf("# benchmark PID: %d\n", bm_pid);
+ ksft_print_msg("benchmark PID: %d\n", bm_pid);
/*
* Register CTRL-C handler for parent, as it has to kill benchmark
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index b47f4f150189..e3d18e113313 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -10,8 +10,6 @@
*/
#include "resctrl.h"
-int tests_run;
-
static int find_resctrl_mount(char *buffer)
{
FILE *mounts;
@@ -68,23 +66,17 @@ int remount_resctrlfs(bool mum_resctrlfs)
if (ret)
strcpy(mountpoint, RESCTRL_PATH);
- if (!ret && mum_resctrlfs && umount(mountpoint)) {
- printf("not ok unmounting \"%s\"\n", mountpoint);
- perror("# umount");
- tests_run++;
- }
+ if (!ret && mum_resctrlfs && umount(mountpoint))
+ ksft_print_msg("fail: unmounting \"%s\"\n", mountpoint);
if (!ret && !mum_resctrlfs)
return 0;
+ ksft_print_msg("mounting resctrl to \"%s\"\n", RESCTRL_PATH);
ret = mount("resctrl", RESCTRL_PATH, "resctrl", 0, NULL);
- printf("%sok mounting resctrl to \"%s\"\n", ret ? "not " : "",
- RESCTRL_PATH);
if (ret)
perror("# mount");
- tests_run++;
-
return ret;
}
@@ -477,13 +469,10 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char *ctrlgrp, char *mongrp,
}
out:
- printf("%sok writing benchmark parameters to resctrl FS\n",
- ret ? "not " : "");
+ ksft_print_msg("writing benchmark parameters to resctrl FS\n");
if (ret)
perror("# writing to resctrlfs");
- tests_run++;
-
return ret;
}
@@ -511,7 +500,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
return -ENOENT;
if (!schemata) {
- printf("# Skipping empty schemata update\n");
+ ksft_print_msg("Skipping empty schemata update\n");
return -1;
}
@@ -552,10 +541,9 @@ int write_schemata(char *ctrlgrp, char *schemata, int cpu_no, char *resctrl_val)
fclose(fp);
out:
- printf("%sok Write schema \"%s\" to resctrl FS%s%s\n",
- ret ? "not " : "", schema, ret ? " # " : "",
- ret ? reason : "");
- tests_run++;
+ ksft_print_msg("Write schema \"%s\" to resctrl FS%s%s\n",
+ schema, ret ? " # " : "",
+ ret ? reason : "");
return ret;
}
@@ -579,18 +567,17 @@ bool check_resctrlfs_support(void)
fclose(inf);
- printf("%sok kernel supports resctrl filesystem\n", ret ? "" : "not ");
- tests_run++;
+ ksft_print_msg("%s kernel supports resctrl filesystem\n",
+ ret ? "pass:" : "fail:");
dp = opendir(RESCTRL_PATH);
- printf("%sok resctrl mountpoint \"%s\" exists\n",
- dp ? "" : "not ", RESCTRL_PATH);
+ ksft_print_msg("%s resctrl mountpoint \"%s\" exists\n",
+ dp ? "pass:" : "fail:", RESCTRL_PATH);
if (dp)
closedir(dp);
- tests_run++;
- printf("# resctrl filesystem %s mounted\n",
- find_resctrl_mount(NULL) ? "not" : "is");
+ ksft_print_msg("resctrl filesystem %s mounted\n",
+ find_resctrl_mount(NULL) ? "not" : "is");
return ret;
}
@@ -672,9 +659,9 @@ int filter_dmesg(void)
while (fgets(line, 1024, fp)) {
if (strstr(line, "intel_rdt:"))
- printf("# dmesg: %s", line);
+ ksft_print_msg("dmesg: %s", line);
if (strstr(line, "resctrl:"))
- printf("# dmesg: %s", line);
+ ksft_print_msg("dmesg: %s", line);
}
fclose(fp);
waitpid(pid, NULL, 0);
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* RE: [PATCH v5 08/21] selftests/resctrl: Call kselftest APIs to log test results
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
0 siblings, 1 reply; 32+ messages in thread
From: Babu Moger @ 2021-03-12 19:12 UTC (permalink / raw)
To: Fenghua Yu, Shuah Khan, Tony Luck, Reinette Chatre
Cc: linux-kselftest, linux-kernel
> -----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 08/21] selftests/resctrl: Call kselftest APIs to log test results
>
> Call kselftest APIs instead of using printf() to log test results
> for cleaner code and better future extension.
>
> Suggested-by: Shuah Khan <shuah@kernel.org>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
> Change Log:
> v5:
> - Add this patch (Shuah)
>
> tools/testing/selftests/resctrl/cat_test.c | 37 +++++++--------
> tools/testing/selftests/resctrl/cmt_test.c | 42 ++++++++---------
> tools/testing/selftests/resctrl/mba_test.c | 24 +++++-----
> tools/testing/selftests/resctrl/mbm_test.c | 28 ++++++------
> tools/testing/selftests/resctrl/resctrl.h | 2 +-
> .../testing/selftests/resctrl/resctrl_tests.c | 40 +++++++++--------
> tools/testing/selftests/resctrl/resctrl_val.c | 4 +-
> tools/testing/selftests/resctrl/resctrlfs.c | 45 +++++++------------
> 8 files changed, 105 insertions(+), 117 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/cat_test.c
> b/tools/testing/selftests/resctrl/cat_test.c
> index 20823725daca..0deb38ed971b 100644
> --- a/tools/testing/selftests/resctrl/cat_test.c
> +++ b/tools/testing/selftests/resctrl/cat_test.c
> @@ -52,25 +52,28 @@ static int cat_setup(int num, ...)
> return ret;
> }
>
> -static void show_cache_info(unsigned long sum_llc_perf_miss, int no_of_bits,
> - unsigned long span)
> +static int show_cache_info(unsigned long sum_llc_perf_miss, int no_of_bits,
> + unsigned long span)
> {
> unsigned long allocated_cache_lines = span / 64;
> unsigned long avg_llc_perf_miss = 0;
> float diff_percent;
> + int ret;
>
> avg_llc_perf_miss = sum_llc_perf_miss / (NUM_OF_RUNS - 1);
> diff_percent = ((float)allocated_cache_lines - avg_llc_perf_miss) /
> allocated_cache_lines * 100;
>
> - printf("%sok CAT: cache miss rate within %d%%\n",
> - !is_amd && abs((int)diff_percent) > MAX_DIFF_PERCENT ?
> - "not " : "", MAX_DIFF_PERCENT);
> - tests_run++;
> - printf("# Percent diff=%d\n", abs((int)diff_percent));
> - printf("# Number of bits: %d\n", no_of_bits);
> - printf("# Avg_llc_perf_miss: %lu\n", avg_llc_perf_miss);
> - printf("# Allocated cache lines: %lu\n", allocated_cache_lines);
> + ret = !is_amd && abs((int)diff_percent) > MAX_DIFF_PERCENT;
> + ksft_print_msg("cache miss rate %swithin %d%%\n",
> + ret ? "not " : "", MAX_DIFF_PERCENT);
> +
> + ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent));
> + ksft_print_msg("Number of bits: %d\n", no_of_bits);
> + ksft_print_msg("Avg_llc_perf_miss: %lu\n", avg_llc_perf_miss);
> + ksft_print_msg("Allocated cache lines: %lu\n", allocated_cache_lines);
> +
> + return ret;
> }
>
> static int check_results(struct resctrl_val_param *param)
> @@ -80,7 +83,7 @@ static int check_results(struct resctrl_val_param *param)
> int runs = 0, no_of_bits = 0;
> FILE *fp;
>
> - printf("# Checking for pass/fail\n");
> + ksft_print_msg("Checking for pass/fail\n");
> fp = fopen(param->filename, "r");
> if (!fp) {
> perror("# Cannot open file");
> @@ -108,9 +111,7 @@ static int check_results(struct resctrl_val_param
> *param)
> fclose(fp);
> no_of_bits = count_bits(param->mask);
>
> - show_cache_info(sum_llc_perf_miss, no_of_bits, param->span);
> -
> - return 0;
> + return show_cache_info(sum_llc_perf_miss, no_of_bits, param->span);
> }
>
> void cat_test_cleanup(void)
> @@ -146,15 +147,15 @@ int cat_perf_miss_val(int cpu_no, int n, char
> *cache_type)
> ret = get_cache_size(cpu_no, cache_type, &cache_size);
> if (ret)
> return ret;
> - printf("cache size :%lu\n", cache_size);
> + ksft_print_msg("cache size :%lu\n", cache_size);
>
> /* Get max number of bits from default-cabm mask */
> count_of_bits = count_bits(long_mask);
>
> if (n < 1 || n > count_of_bits - 1) {
> - printf("Invalid input value for no_of_bits n!\n");
> - printf("Please Enter value in range 1 to %d\n",
> - count_of_bits - 1);
> + ksft_print_msg("Invalid input value for no_of_bits n!\n");
> + ksft_print_msg("Please Enter value in range 1 to %d\n",
> + count_of_bits - 1);
> return -1;
> }
>
> diff --git a/tools/testing/selftests/resctrl/cmt_test.c
> b/tools/testing/selftests/resctrl/cmt_test.c
> index ca82db37c1f7..e5af19335115 100644
> --- a/tools/testing/selftests/resctrl/cmt_test.c
> +++ b/tools/testing/selftests/resctrl/cmt_test.c
> @@ -39,36 +39,33 @@ static int cmt_setup(int num, ...)
> return 0;
> }
>
> -static void show_cache_info(unsigned long sum_llc_occu_resc, int no_of_bits,
> - unsigned long span)
> +static int show_cache_info(unsigned long sum_llc_occu_resc, int no_of_bits,
> + unsigned long span)
> {
> unsigned long avg_llc_occu_resc = 0;
> float diff_percent;
> long avg_diff = 0;
> - bool res;
> + int ret;
>
> avg_llc_occu_resc = sum_llc_occu_resc / (NUM_OF_RUNS - 1);
> avg_diff = (long)abs(span - avg_llc_occu_resc);
>
> diff_percent = (((float)span - avg_llc_occu_resc) / span) * 100;
>
> - if ((abs((int)diff_percent) <= MAX_DIFF_PERCENT) ||
> - (abs(avg_diff) <= MAX_DIFF))
> - res = true;
> - else
> - res = false;
> + ret = (abs((int)diff_percent) > MAX_DIFF_PERCENT) &&
> + (abs(avg_diff) > MAX_DIFF);
>
> - printf("%sok CMT: diff within %d, %d\%%\n", res ? "" : "not",
> - MAX_DIFF, (int)MAX_DIFF_PERCENT);
> + ksft_print_msg("%scache miss diff within %d, %d\%%\n",
> + ret ? "fail: " : "not", MAX_DIFF, (int)MAX_DIFF_PERCENT);
>
> - printf("# diff: %ld\n", avg_diff);
> - printf("# percent diff=%d\n", abs((int)diff_percent));
> - printf("# Results are displayed in (Bytes)\n");
> - printf("# Number of bits: %d\n", no_of_bits);
> - printf("# Avg_llc_occu_resc: %lu\n", avg_llc_occu_resc);
> - printf("# llc_occu_exp (span): %lu\n", span);
> + ksft_print_msg("diff: %ld\n", avg_diff);
> + ksft_print_msg("percent diff=%d\n", abs((int)diff_percent));
> + ksft_print_msg("Results are displayed in (Bytes)\n");
> + ksft_print_msg("Number of bits: %d\n", no_of_bits);
> + ksft_print_msg("Avg_llc_occu_resc: %lu\n", avg_llc_occu_resc);
> + ksft_print_msg("llc_occu_exp (span): %lu\n", span);
>
> - tests_run++;
> + return ret;
> }
>
> static int check_results(struct resctrl_val_param *param, int no_of_bits)
> @@ -78,7 +75,7 @@ static int check_results(struct resctrl_val_param *param,
> int no_of_bits)
> int runs = 0;
> FILE *fp;
>
> - printf("# checking for pass/fail\n");
> + ksft_print_msg("checking for pass/fail\n");
> fp = fopen(param->filename, "r");
> if (!fp) {
> perror("# Error in opening file\n");
> @@ -101,9 +98,8 @@ static int check_results(struct resctrl_val_param *param,
> int no_of_bits)
> runs++;
> }
> fclose(fp);
> - show_cache_info(sum_llc_occu_resc, no_of_bits, param->span);
>
> - return 0;
> + return show_cache_info(sum_llc_occu_resc, no_of_bits, param->span);
> }
>
> void cmt_test_cleanup(void)
> @@ -134,13 +130,13 @@ int cmt_resctrl_val(int cpu_no, int n, char
> **benchmark_cmd)
> ret = get_cache_size(cpu_no, "L3", &cache_size);
> if (ret)
> return ret;
> - printf("cache size :%lu\n", cache_size);
> + ksft_print_msg("cache size :%lu\n", cache_size);
>
> count_of_bits = count_bits(long_mask);
>
> if (n < 1 || n > count_of_bits) {
> - printf("Invalid input value for numbr_of_bits n!\n");
> - printf("Please Enter value in range 1 to %d\n", count_of_bits);
> + ksft_print_msg("Invalid input value for numbr_of_bits n!\n");
> + ksft_print_msg("Please Enter value in range 1 to %d\n",
> count_of_bits);
> return -1;
> }
>
> diff --git a/tools/testing/selftests/resctrl/mba_test.c
> b/tools/testing/selftests/resctrl/mba_test.c
> index 6449fbd96096..3a226effe80c 100644
> --- a/tools/testing/selftests/resctrl/mba_test.c
> +++ b/tools/testing/selftests/resctrl/mba_test.c
> @@ -56,7 +56,7 @@ static void show_mba_info(unsigned long *bw_imc,
> unsigned long *bw_resc)
> int allocation, runs;
> bool failed = false;
>
> - printf("# Results are displayed in (MB)\n");
> + ksft_print_msg("Results are displayed in (MB)\n");
> /* Memory bandwidth from 100% down to 10% */
> for (allocation = 0; allocation < ALLOCATION_MAX / ALLOCATION_STEP;
> allocation++) {
> @@ -78,21 +78,21 @@ static void show_mba_info(unsigned long *bw_imc,
> unsigned long *bw_resc)
> avg_bw_resc = sum_bw_resc / (NUM_OF_RUNS - 1);
> avg_diff = labs((long)(avg_bw_resc - avg_bw_imc));
>
> - printf("%sok MBA schemata percentage %u smaller than %d
> %%\n",
> - avg_diff > MAX_DIFF ? "not " : "",
> - ALLOCATION_MAX - ALLOCATION_STEP * allocation,
> - MAX_DIFF);
> - tests_run++;
> - printf("# avg_diff: %lu\n", avg_diff);
> - printf("# avg_bw_imc: %lu\n", avg_bw_imc);
> - printf("# avg_bw_resc: %lu\n", avg_bw_resc);
> + ksft_print_msg("%s MBA schemata percentage %u smaller than
> %d %%\n",
> + avg_diff > MAX_DIFF ? "fail:" : "pass:",
> + ALLOCATION_MAX - ALLOCATION_STEP *
> allocation,
> + MAX_DIFF);
> + ksft_print_msg("avg_diff: %lu\n", avg_diff);
> + ksft_print_msg("avg_bw_imc: %lu\n", avg_bw_imc);
> + ksft_print_msg("avg_bw_resc: %lu\n", avg_bw_resc);
> if (avg_diff > MAX_DIFF)
> failed = true;
> }
>
> - printf("%sok schemata change using MBA%s\n", failed ? "not " : "",
> - failed ? " # at least one test failed" : "");
> - tests_run++;
> + ksft_print_msg("%s schemata change using MBA\n",
> + failed ? "fail:" : "pass:");
> + if (failed)
> + ksft_print_msg("at least one test failed");
> }
>
> static int check_results(void)
> diff --git a/tools/testing/selftests/resctrl/mbm_test.c
> b/tools/testing/selftests/resctrl/mbm_test.c
> index ec6cfe01c9c2..2b4f26013d84 100644
> --- a/tools/testing/selftests/resctrl/mbm_test.c
> +++ b/tools/testing/selftests/resctrl/mbm_test.c
> @@ -14,13 +14,13 @@
> #define MAX_DIFF 300
> #define NUM_OF_RUNS 5
>
> -static void
> +static int
> show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span)
> {
> unsigned long avg_bw_imc = 0, avg_bw_resc = 0;
> unsigned long sum_bw_imc = 0, sum_bw_resc = 0;
> long avg_diff = 0;
> - int runs;
> + int runs, ret;
>
> /*
> * Discard the first value which is inaccurate due to monitoring setup
> @@ -35,13 +35,15 @@ show_bw_info(unsigned long *bw_imc, unsigned long
> *bw_resc, int span)
> avg_bw_resc = sum_bw_resc / 4;
> avg_diff = avg_bw_resc - avg_bw_imc;
>
> - printf("%sok MBM: diff within %d%%\n",
> - labs(avg_diff) > MAX_DIFF ? "not " : "", MAX_DIFF);
> - tests_run++;
> - printf("# avg_diff: %lu\n", labs(avg_diff));
> - printf("# Span (MB): %d\n", span);
> - printf("# avg_bw_imc: %lu\n", avg_bw_imc);
> - printf("# avg_bw_resc: %lu\n", avg_bw_resc);
> + ret = labs(avg_diff) > MAX_DIFF;
> + ksft_print_msg("%s MBM: diff within %d%%\n",
> + ret ? "fail:" : "pass:", MAX_DIFF);
> + ksft_print_msg("avg_diff: %lu\n", labs(avg_diff));
> + ksft_print_msg("Span (MB): %d\n", span);
> + ksft_print_msg("avg_bw_imc: %lu\n", avg_bw_imc);
> + ksft_print_msg("avg_bw_resc: %lu\n", avg_bw_resc);
> +
> + return ret;
> }
>
> static int check_results(int span)
> @@ -49,10 +51,10 @@ static int check_results(int span)
> unsigned long bw_imc[NUM_OF_RUNS], bw_resc[NUM_OF_RUNS];
> char temp[1024], *token_array[8];
> char output[] = RESULT_FILE_NAME;
> - int runs;
> + int runs, ret;
> FILE *fp;
>
> - printf("# Checking for pass/fail\n");
> + ksft_print_msg("Checking for pass/fail\n");
>
> fp = fopen(output, "r");
> if (!fp) {
> @@ -76,11 +78,11 @@ static int check_results(int span)
> runs++;
> }
>
> - show_bw_info(bw_imc, bw_resc, span);
> + ret = show_bw_info(bw_imc, bw_resc, span);
>
> fclose(fp);
>
> - return 0;
> + return ret;
> }
>
> static int mbm_setup(int num, ...)
> diff --git a/tools/testing/selftests/resctrl/resctrl.h
> b/tools/testing/selftests/resctrl/resctrl.h
> index 1a58767a0bd2..ebf88217f9de 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -23,6 +23,7 @@
> #include <sys/eventfd.h>
> #include <asm/unistd.h>
> #include <linux/perf_event.h>
> +#include "../kselftest.h"
>
> #define MB (1024 * 1024)
> #define RESCTRL_PATH "/sys/fs/resctrl"
> @@ -68,7 +69,6 @@ struct resctrl_val_param {
> #define CAT_STR "cat"
>
> extern pid_t bm_pid, ppid;
> -extern int tests_run;
>
> extern char llc_occup_path[1024];
> extern bool is_amd;
> diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c
> b/tools/testing/selftests/resctrl/resctrl_tests.c
> index 01f7aed7f54e..ebc24992cc2c 100644
> --- a/tools/testing/selftests/resctrl/resctrl_tests.c
> +++ b/tools/testing/selftests/resctrl/resctrl_tests.c
> @@ -60,7 +60,7 @@ int main(int argc, char **argv)
> int res, c, cpu_no = 1, span = 250, argc_new = argc, i, no_of_bits = 5;
> char *benchmark_cmd[BENCHMARK_ARGS], bw_report[64],
> bm_type[64];
> char
> benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE];
> - int ben_ind, ben_count;
> + int ben_ind, ben_count, tests = 0;
> bool cat_test = true;
>
> for (i = 0; i < argc; i++) {
> @@ -87,12 +87,16 @@ int main(int argc, char **argv)
> while (token) {
> if (!strncmp(token, MBM_STR,
> sizeof(MBM_STR))) {
> mbm_test = true;
> + tests++;
> } else if (!strncmp(token, MBA_STR,
> sizeof(MBA_STR))) {
> mba_test = true;
> + tests++;
> } else if (!strncmp(token, CMT_STR,
> sizeof(CMT_STR))) {
> cmt_test = true;
> + tests++;
> } else if (!strncmp(token, CAT_STR,
> sizeof(CAT_STR))) {
> cat_test = true;
> + tests++;
> } else {
> printf("invalid argument\n");
>
> @@ -118,7 +122,7 @@ int main(int argc, char **argv)
> }
> }
>
> - printf("TAP version 13\n");
> + ksft_print_header();
>
> /*
> * Typically we need root privileges, because:
> @@ -126,7 +130,7 @@ int main(int argc, char **argv)
> * 2. We execute perf commands
> */
> if (geteuid() != 0)
> - printf("# WARNING: not running as root, tests may fail.\n");
> + return ksft_exit_fail_msg("not running as root, abort
> testing.\n");
>
> /* Detect AMD vendor */
> detect_amd();
> @@ -155,48 +159,46 @@ int main(int argc, char **argv)
> sprintf(bw_report, "reads");
> sprintf(bm_type, "fill_buf");
>
> - check_resctrlfs_support();
> + if (!check_resctrlfs_support())
> + return ksft_exit_fail_msg("resctrl FS does not exist\n");
> +
> filter_dmesg();
>
> + ksft_set_plan(tests ? : 4);
> +
> if (!is_amd && mbm_test) {
> - printf("# Starting MBM BW change ...\n");
> + ksft_print_msg("Starting MBM BW change ...\n");
> if (!has_ben)
> sprintf(benchmark_cmd[5], "%s", MBA_STR);
> res = mbm_bw_change(span, cpu_no, bw_report,
> benchmark_cmd);
> - printf("%sok MBM: bw change\n", res ? "not " : "");
> + ksft_test_result(!res, "MBM: bw change\n");
> mbm_test_cleanup();
> - tests_run++;
> }
>
> if (!is_amd && mba_test) {
> - printf("# Starting MBA Schemata change ...\n");
> + ksft_print_msg("Starting MBA Schemata change ...\n");
> if (!has_ben)
> sprintf(benchmark_cmd[1], "%d", span);
> res = mba_schemata_change(cpu_no, bw_report,
> benchmark_cmd);
> - printf("%sok MBA: schemata change\n", res ? "not " : "");
> + ksft_test_result(!res, "MBA: schemata change\n");
> mba_test_cleanup();
> - tests_run++;
> }
>
> if (cmt_test) {
> - printf("# Starting CMT test ...\n");
> + ksft_print_msg("Starting CMT test ...\n");
> if (!has_ben)
> sprintf(benchmark_cmd[5], "%s", "cmt");
> res = cmt_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
> - printf("%sok CMT: test\n", res ? "not " : "");
> + ksft_test_result(!res, "CMT: test\n");
> cmt_test_cleanup();
> - tests_run++;
> }
>
> if (cat_test) {
> - printf("# Starting CAT test ...\n");
> + ksft_print_msg("Starting CAT test ...\n");
> res = cat_perf_miss_val(cpu_no, no_of_bits, "L3");
> - printf("%sok CAT: test\n", res ? "not " : "");
> - tests_run++;
> + ksft_test_result(!res, "CAT: test\n");
> cat_test_cleanup();
> }
>
> - printf("1..%d\n", tests_run);
> -
> - return 0;
> + return ksft_exit_pass();
> }
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c
> b/tools/testing/selftests/resctrl/resctrl_val.c
> index 17770095c98e..48bcd5fd7d79 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -449,7 +449,7 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr)
> kill(bm_pid, SIGKILL);
> umount_resctrlfs();
> tests_cleanup();
> - printf("Ending\n\n");
> + ksft_print_msg("Ending\n\n");
>
> exit(EXIT_SUCCESS);
> }
> @@ -645,7 +645,7 @@ int resctrl_val(char **benchmark_cmd, struct
> resctrl_val_param *param)
> PARENT_EXIT("Child is done");
> }
>
> - printf("# benchmark PID: %d\n", bm_pid);
> + ksft_print_msg("benchmark PID: %d\n", bm_pid);
>
> /*
> * Register CTRL-C handler for parent, as it has to kill benchmark
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c
> b/tools/testing/selftests/resctrl/resctrlfs.c
> index b47f4f150189..e3d18e113313 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -10,8 +10,6 @@
> */
> #include "resctrl.h"
>
> -int tests_run;
> -
> static int find_resctrl_mount(char *buffer)
> {
> FILE *mounts;
> @@ -68,23 +66,17 @@ int remount_resctrlfs(bool mum_resctrlfs)
> if (ret)
> strcpy(mountpoint, RESCTRL_PATH);
>
> - if (!ret && mum_resctrlfs && umount(mountpoint)) {
> - printf("not ok unmounting \"%s\"\n", mountpoint);
> - perror("# umount");
> - tests_run++;
> - }
> + if (!ret && mum_resctrlfs && umount(mountpoint))
> + ksft_print_msg("fail: unmounting \"%s\"\n", mountpoint);
>
> if (!ret && !mum_resctrlfs)
> return 0;
>
> + ksft_print_msg("mounting resctrl to \"%s\"\n", RESCTRL_PATH);
> ret = mount("resctrl", RESCTRL_PATH, "resctrl", 0, NULL);
> - printf("%sok mounting resctrl to \"%s\"\n", ret ? "not " : "",
> - RESCTRL_PATH);
> if (ret)
> perror("# mount");
>
> - tests_run++;
> -
> return ret;
> }
>
> @@ -477,13 +469,10 @@ int write_bm_pid_to_resctrl(pid_t bm_pid, char
> *ctrlgrp, char *mongrp,
> }
>
> out:
> - printf("%sok writing benchmark parameters to resctrl FS\n",
> - ret ? "not " : "");
> + ksft_print_msg("writing benchmark parameters to resctrl FS\n");
> if (ret)
> perror("# writing to resctrlfs");
>
> - tests_run++;
> -
> return ret;
> }
>
> @@ -511,7 +500,7 @@ int write_schemata(char *ctrlgrp, char *schemata, int
> cpu_no, char *resctrl_val)
> return -ENOENT;
>
> if (!schemata) {
> - printf("# Skipping empty schemata update\n");
> + ksft_print_msg("Skipping empty schemata update\n");
>
> return -1;
> }
> @@ -552,10 +541,9 @@ int write_schemata(char *ctrlgrp, char *schemata, int
> cpu_no, char *resctrl_val)
> fclose(fp);
>
> out:
> - printf("%sok Write schema \"%s\" to resctrl FS%s%s\n",
> - ret ? "not " : "", schema, ret ? " # " : "",
> - ret ? reason : "");
> - tests_run++;
> + ksft_print_msg("Write schema \"%s\" to resctrl FS%s%s\n",
> + schema, ret ? " # " : "",
> + ret ? reason : "");
>
> return ret;
> }
> @@ -579,18 +567,17 @@ bool check_resctrlfs_support(void)
>
> fclose(inf);
>
> - printf("%sok kernel supports resctrl filesystem\n", ret ? "" : "not ");
> - tests_run++;
> + ksft_print_msg("%s kernel supports resctrl filesystem\n",
> + ret ? "pass:" : "fail:");
>
> dp = opendir(RESCTRL_PATH);
> - printf("%sok resctrl mountpoint \"%s\" exists\n",
> - dp ? "" : "not ", RESCTRL_PATH);
> + ksft_print_msg("%s resctrl mountpoint \"%s\" exists\n",
> + dp ? "pass:" : "fail:", RESCTRL_PATH);
> if (dp)
> closedir(dp);
> - tests_run++;
>
> - printf("# resctrl filesystem %s mounted\n",
> - find_resctrl_mount(NULL) ? "not" : "is");
> + ksft_print_msg("resctrl filesystem %s mounted\n",
> + find_resctrl_mount(NULL) ? "not" : "is");
>
> return ret;
> }
> @@ -672,9 +659,9 @@ int filter_dmesg(void)
>
> while (fgets(line, 1024, fp)) {
> if (strstr(line, "intel_rdt:"))
> - printf("# dmesg: %s", line);
> + ksft_print_msg("dmesg: %s", line);
> if (strstr(line, "resctrl:"))
> - printf("# dmesg: %s", line);
> + ksft_print_msg("dmesg: %s", line);
> }
> fclose(fp);
> waitpid(pid, NULL, 0);
> --
> 2.30.1
In general, this patch has some minor nits. When displaying the messages,
normally the first character should be capitalized.
ksft_print_msg("checking for pass/fail\n");
should be
ksft_print_msg("Checking for pass/fail\n");
And
ksft_print_msg("Please Enter value in range 1 to %d\n",count_of_bits);
Should be
ksft_print_msg("Please enter value in range 1 to %d\n", count_of_bits);
I am not too concerned about this. You can improve it if you like it.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 08/21] selftests/resctrl: Call kselftest APIs to log test results
2021-03-12 19:12 ` Babu Moger
@ 2021-03-12 22:09 ` Fenghua Yu
0 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-12 22:09 UTC (permalink / raw)
To: Babu Moger
Cc: Shuah Khan, Tony Luck, Reinette Chatre, linux-kselftest, linux-kernel
Hi, Babu,
On Fri, Mar 12, 2021 at 01:12:35PM -0600, Babu Moger wrote:
> > - printf("# dmesg: %s", line);
> > + ksft_print_msg("dmesg: %s", line);
> > if (strstr(line, "resctrl:"))
> > - printf("# dmesg: %s", line);
> > + ksft_print_msg("dmesg: %s", line);
>
> In general, this patch has some minor nits. When displaying the messages,
> normally the first character should be capitalized.
> ksft_print_msg("checking for pass/fail\n");
> should be
> ksft_print_msg("Checking for pass/fail\n");
>
> And
> ksft_print_msg("Please Enter value in range 1 to %d\n",count_of_bits);
> Should be
>
> ksft_print_msg("Please enter value in range 1 to %d\n", count_of_bits);
>
> I am not too concerned about this. You can improve it if you like it.
>
Ok. Will fix them.
Thanks.
-Fenghua
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v5 09/21] selftests/resctrl: Share show_cache_info() by CAT and CMT tests
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (7 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 08/21] selftests/resctrl: Call kselftest APIs to log test results Fenghua Yu
@ 2021-03-07 14:54 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 10/21] selftests/resctrl: Fix a printed message Fenghua Yu
` (12 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
show_cache_info() functions are defined separately in CAT and CMT
tests. But the functions are same for the tests and unnecessary
to be defined separately. Share the function by the tests.
Suggested-by: Shuah Khan <shuah@kernel.org>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Add this patch (Shuah)
tools/testing/selftests/resctrl/cache.c | 42 ++++++++++++++++++++++
tools/testing/selftests/resctrl/cat_test.c | 28 ++-------------
tools/testing/selftests/resctrl/cmt_test.c | 33 ++---------------
tools/testing/selftests/resctrl/resctrl.h | 4 +++
4 files changed, 52 insertions(+), 55 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 2aa1b5c7d9e1..eaf5116b112c 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -270,3 +270,45 @@ int cat_val(struct resctrl_val_param *param)
return ret;
}
+
+/*
+ * show_cache_info: show cache test result information
+ * @sum_llc_val: sum of LLC cache result data
+ * @no_of_bits: number of bits
+ * @cache_span: cache span in bytes for CMT or in lines for CAT
+ * @max_diff: max difference
+ * @max_diff_percent: max difference percentage
+ * @num_of_runs: number of runs
+ * @platform: show test information on this platform
+ * @cmt: CMT test or CAT test
+ *
+ * Return: 0 on success. non-zero on failure.
+ */
+int show_cache_info(unsigned long sum_llc_val, int no_of_bits,
+ unsigned long cache_span, unsigned long max_diff,
+ unsigned long max_diff_percent, unsigned long num_of_runs,
+ bool platform, bool cmt)
+{
+ unsigned long avg_llc_val = 0;
+ float diff_percent;
+ long avg_diff = 0;
+ int ret;
+
+ avg_llc_val = sum_llc_val / (num_of_runs - 1);
+ avg_diff = (long)abs(cache_span - avg_llc_val);
+ diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100;
+
+ ret = platform && abs((int)diff_percent) > max_diff_percent &&
+ (cmt ? (abs(avg_diff) > max_diff) : true);
+
+ ksft_print_msg("%scache miss rate within %d%%\n",
+ ret ? "fail: " : "", max_diff_percent);
+
+ ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent));
+ ksft_print_msg("Number of bits: %d\n", no_of_bits);
+ ksft_print_msg("Average LLC val: %lu\n", avg_llc_val);
+ ksft_print_msg("Cache span (%s): %lu\n", cmt ? "bytes" : "lines",
+ cache_span);
+
+ return ret;
+}
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 0deb38ed971b..109363e9a7d7 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -52,30 +52,6 @@ static int cat_setup(int num, ...)
return ret;
}
-static int show_cache_info(unsigned long sum_llc_perf_miss, int no_of_bits,
- unsigned long span)
-{
- unsigned long allocated_cache_lines = span / 64;
- unsigned long avg_llc_perf_miss = 0;
- float diff_percent;
- int ret;
-
- avg_llc_perf_miss = sum_llc_perf_miss / (NUM_OF_RUNS - 1);
- diff_percent = ((float)allocated_cache_lines - avg_llc_perf_miss) /
- allocated_cache_lines * 100;
-
- ret = !is_amd && abs((int)diff_percent) > MAX_DIFF_PERCENT;
- ksft_print_msg("cache miss rate %swithin %d%%\n",
- ret ? "not " : "", MAX_DIFF_PERCENT);
-
- ksft_print_msg("Percent diff=%d\n", abs((int)diff_percent));
- ksft_print_msg("Number of bits: %d\n", no_of_bits);
- ksft_print_msg("Avg_llc_perf_miss: %lu\n", avg_llc_perf_miss);
- ksft_print_msg("Allocated cache lines: %lu\n", allocated_cache_lines);
-
- return ret;
-}
-
static int check_results(struct resctrl_val_param *param)
{
char *token_array[8], temp[512];
@@ -111,7 +87,9 @@ static int check_results(struct resctrl_val_param *param)
fclose(fp);
no_of_bits = count_bits(param->mask);
- return show_cache_info(sum_llc_perf_miss, no_of_bits, param->span);
+ return show_cache_info(sum_llc_perf_miss, no_of_bits, param->span / 64,
+ MAX_DIFF, MAX_DIFF_PERCENT, NUM_OF_RUNS,
+ !is_amd, false);
}
void cat_test_cleanup(void)
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index e5af19335115..4adb92cb6ca1 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -39,35 +39,6 @@ static int cmt_setup(int num, ...)
return 0;
}
-static int show_cache_info(unsigned long sum_llc_occu_resc, int no_of_bits,
- unsigned long span)
-{
- unsigned long avg_llc_occu_resc = 0;
- float diff_percent;
- long avg_diff = 0;
- int ret;
-
- avg_llc_occu_resc = sum_llc_occu_resc / (NUM_OF_RUNS - 1);
- avg_diff = (long)abs(span - avg_llc_occu_resc);
-
- diff_percent = (((float)span - avg_llc_occu_resc) / span) * 100;
-
- ret = (abs((int)diff_percent) > MAX_DIFF_PERCENT) &&
- (abs(avg_diff) > MAX_DIFF);
-
- ksft_print_msg("%scache miss diff within %d, %d\%%\n",
- ret ? "fail: " : "not", MAX_DIFF, (int)MAX_DIFF_PERCENT);
-
- ksft_print_msg("diff: %ld\n", avg_diff);
- ksft_print_msg("percent diff=%d\n", abs((int)diff_percent));
- ksft_print_msg("Results are displayed in (Bytes)\n");
- ksft_print_msg("Number of bits: %d\n", no_of_bits);
- ksft_print_msg("Avg_llc_occu_resc: %lu\n", avg_llc_occu_resc);
- ksft_print_msg("llc_occu_exp (span): %lu\n", span);
-
- return ret;
-}
-
static int check_results(struct resctrl_val_param *param, int no_of_bits)
{
char *token_array[8], temp[512];
@@ -99,7 +70,9 @@ static int check_results(struct resctrl_val_param *param, int no_of_bits)
}
fclose(fp);
- return show_cache_info(sum_llc_occu_resc, no_of_bits, param->span);
+ return show_cache_info(sum_llc_occu_resc, no_of_bits, param->span,
+ MAX_DIFF, MAX_DIFF_PERCENT, NUM_OF_RUNS,
+ true, true);
}
void cmt_test_cleanup(void)
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index ebf88217f9de..81f322245ef7 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -108,5 +108,9 @@ unsigned int count_bits(unsigned long n);
void cmt_test_cleanup(void);
int get_core_sibling(int cpu_no);
int measure_cache_vals(struct resctrl_val_param *param, int bm_pid);
+int show_cache_info(unsigned long sum_llc_val, int no_of_bits,
+ unsigned long cache_span, unsigned long max_diff,
+ unsigned long max_diff_percent, unsigned long num_of_runs,
+ bool platform, bool cmt);
#endif /* RESCTRL_H */
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 10/21] selftests/resctrl: Fix a printed message
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (8 preceding siblings ...)
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 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 11/21] selftests/resctrl: Add config dependencies Fenghua Yu
` (11 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
From: Reinette Chatre <reinette.chatre@intel.com>
Add a missing newline to the printed help text to improve readability.
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Remove the "notok" fix part because the API change fixes it already.
tools/testing/selftests/resctrl/resctrl_tests.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index ebc24992cc2c..f1b08afbc3d0 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -37,8 +37,8 @@ void detect_amd(void)
static void cmd_help(void)
{
printf("usage: resctrl_tests [-h] [-b \"benchmark_cmd [options]\"] [-t test list] [-n no_of_bits]\n");
- printf("\t-b benchmark_cmd [options]: run specified benchmark for MBM, MBA and CMT");
- printf("\t default benchmark is builtin fill_buf\n");
+ printf("\t-b benchmark_cmd [options]: run specified benchmark for MBM, MBA and CMT\n");
+ printf("\t default benchmark is builtin fill_buf\n");
printf("\t-t test list: run tests specified in the test list, ");
printf("e.g. -t mbm, mba, cmt, cat\n");
printf("\t-n no_of_bits: run cache tests using specified no of bits in cache bit mask\n");
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 11/21] selftests/resctrl: Add config dependencies
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (9 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 10/21] selftests/resctrl: Fix a printed message Fenghua Yu
@ 2021-03-07 14:54 ` 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
` (10 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
Add the config file for test dependencies.
Suggested-by: Shuah Khan <shuah@kernel.org>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Add this patch (Shuah)
tools/testing/selftests/resctrl/config | 2 ++
1 file changed, 2 insertions(+)
create mode 100644 tools/testing/selftests/resctrl/config
diff --git a/tools/testing/selftests/resctrl/config b/tools/testing/selftests/resctrl/config
new file mode 100644
index 000000000000..8d9f2deb56ed
--- /dev/null
+++ b/tools/testing/selftests/resctrl/config
@@ -0,0 +1,2 @@
+CONFIG_X86_CPU_RESCTRL=y
+CONFIG_PROC_CPU_RESCTRL=y
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 12/21] selftests/resctrl: Check for resctrl mount point only if resctrl FS is supported
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (10 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 11/21] selftests/resctrl: Add config dependencies Fenghua Yu
@ 2021-03-07 14:54 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 13/21] selftests/resctrl: Use resctrl/info for feature detection Fenghua Yu
` (9 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
check_resctrlfs_support() does the following
1. Checks if the platform supports resctrl file system or not by looking
for resctrl in /proc/filesystems
2. Calls opendir() on default resctrl file system path
(i.e. /sys/fs/resctrl)
3. Checks if resctrl file system is mounted or not by looking at
/proc/mounts
Steps 2 and 3 will fail if the platform does not support resctrl file
system. So, there is no need to check for them if step 1 fails.
Fix this by returning immediately if the platform does not support
resctrl file system.
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/resctrlfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index e3d18e113313..10b9292f33e5 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -570,6 +570,9 @@ bool check_resctrlfs_support(void)
ksft_print_msg("%s kernel supports resctrl filesystem\n",
ret ? "pass:" : "fail:");
+ if (!ret)
+ return ret;
+
dp = opendir(RESCTRL_PATH);
ksft_print_msg("%s resctrl mountpoint \"%s\" exists\n",
dp ? "pass:" : "fail:", RESCTRL_PATH);
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 13/21] selftests/resctrl: Use resctrl/info for feature detection
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (11 preceding siblings ...)
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 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 14/21] selftests/resctrl: Fix MBA/MBM results reporting format Fenghua Yu
` (8 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
Resctrl test suite before running any unit test (like cmt, cat, mbm and
mba) should first check if the feature is enabled (by kernel and not just
supported by H/W) on the platform or not.
validate_resctrl_feature_request() is supposed to do that. This function
intends to grep for relevant flags in /proc/cpuinfo but there are several
issues here
1. validate_resctrl_feature_request() calls fgrep() to get flags from
/proc/cpuinfo. But, fgrep() can only return a string with maximum of 255
characters and hence the complete cpu flags are never returned.
2. The substring search logic is also busted. If strstr() finds requested
resctrl feature in the cpu flags, it returns pointer to the first
occurrence. But, the logic negates the return value of strstr() and
hence validate_resctrl_feature_request() returns false if the feature is
present in the cpu flags and returns true if the feature is not present.
3. validate_resctrl_feature_request() checks if a resctrl feature is
reported in /proc/cpuinfo flags or not. Having a cpu flag means that the
H/W supports the feature, but it doesn't mean that the kernel enabled
it. A user could selectively enable only a subset of resctrl features
using kernel command line arguments. Hence, /proc/cpuinfo isn't a
reliable source to check if a feature is enabled or not.
The 3rd issue being the major one and fixing it requires changing the way
validate_resctrl_feature_request() works. Since, /proc/cpuinfo isn't the
right place to check if a resctrl feature is enabled or not, a more
appropriate place is /sys/fs/resctrl/info directory. Change
validate_resctrl_feature_request() such that,
1. For cat, check if /sys/fs/resctrl/info/L3 directory is present or not
2. For mba, check if /sys/fs/resctrl/info/MB directory is present or not
3. For cmt, check if /sys/fs/resctrl/info/L3_MON directory is present and
check if /sys/fs/resctrl/info/L3_MON/mon_features has llc_occupancy
4. For mbm, check if /sys/fs/resctrl/info/L3_MON directory is present and
check if /sys/fs/resctrl/info/L3_MON/mon_features has
mbm_<total/local>_bytes
Please note that only L3_CAT, L3_CMT, MBA and MBM are supported. CDP and L2
variants can be added later.
Reported-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/resctrl.h | 6 ++-
tools/testing/selftests/resctrl/resctrlfs.c | 52 ++++++++++++++++-----
2 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 81f322245ef7..1ad10c47e31d 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -29,6 +29,10 @@
#define RESCTRL_PATH "/sys/fs/resctrl"
#define PHYS_ID_PATH "/sys/devices/system/cpu/cpu"
#define CBM_MASK_PATH "/sys/fs/resctrl/info"
+#define L3_PATH "/sys/fs/resctrl/info/L3"
+#define MB_PATH "/sys/fs/resctrl/info/MB"
+#define L3_MON_PATH "/sys/fs/resctrl/info/L3_MON"
+#define L3_MON_FEATURES_PATH "/sys/fs/resctrl/info/L3_MON/mon_features"
#define PARENT_EXIT(err_msg) \
do { \
@@ -79,7 +83,7 @@ int remount_resctrlfs(bool mum_resctrlfs);
int get_resource_id(int cpu_no, int *resource_id);
int umount_resctrlfs(void);
int validate_bw_report_request(char *bw_report);
-bool validate_resctrl_feature_request(char *resctrl_val);
+bool validate_resctrl_feature_request(const char *resctrl_val);
char *fgrep(FILE *inf, const char *str);
int taskset_benchmark(pid_t bm_pid, int cpu_no);
void run_benchmark(int signum, siginfo_t *info, void *ucontext);
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 10b9292f33e5..0c23514760dd 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -606,26 +606,56 @@ char *fgrep(FILE *inf, const char *str)
* validate_resctrl_feature_request - Check if requested feature is valid.
* @resctrl_val: Requested feature
*
- * Return: 0 on success, non-zero on failure
+ * Return: True if the feature is supported, else false
*/
-bool validate_resctrl_feature_request(char *resctrl_val)
+bool validate_resctrl_feature_request(const char *resctrl_val)
{
- FILE *inf = fopen("/proc/cpuinfo", "r");
+ struct stat statbuf;
bool found = false;
char *res;
+ FILE *inf;
- if (!inf)
+ if (!resctrl_val)
return false;
- res = fgrep(inf, "flags");
-
- if (res) {
- char *s = strchr(res, ':');
+ if (remount_resctrlfs(false))
+ return false;
- found = s && !strstr(s, resctrl_val);
- free(res);
+ if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) {
+ if (!stat(L3_PATH, &statbuf))
+ return true;
+ } else if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
+ if (!stat(MB_PATH, &statbuf))
+ return true;
+ } else if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
+ !strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {
+ if (!stat(L3_MON_PATH, &statbuf)) {
+ inf = fopen(L3_MON_FEATURES_PATH, "r");
+ if (!inf)
+ return false;
+
+ if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {
+ res = fgrep(inf, "llc_occupancy");
+ if (res) {
+ found = true;
+ free(res);
+ }
+ }
+
+ if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR))) {
+ res = fgrep(inf, "mbm_total_bytes");
+ if (res) {
+ free(res);
+ res = fgrep(inf, "mbm_local_bytes");
+ if (res) {
+ found = true;
+ free(res);
+ }
+ }
+ }
+ fclose(inf);
+ }
}
- fclose(inf);
return found;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 14/21] selftests/resctrl: Fix MBA/MBM results reporting format
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (12 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 13/21] selftests/resctrl: Use resctrl/info for feature detection Fenghua Yu
@ 2021-03-07 14:54 ` 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
` (7 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
MBM unit test starts fill_buf (default built-in benchmark) in a new con_mon
group (c1, m1) and records resctrl reported mbm values and iMC (Integrated
Memory Controller) values every second. It does this for five seconds
(randomly chosen value) in total. It then calculates average of resctrl_mbm
values and imc_mbm values and if the difference is greater than 300 MB/sec
(randomly chosen value), the test treats it as a failure. MBA unit test is
similar to MBM but after every run it changes schemata.
Checking for a difference of 300 MB/sec doesn't look very meaningful when
the mbm values are changing over a wide range. For example, below are the
values running MBA test on SKL with different allocations
1. With 10% as schemata both iMC and resctrl mbm_values are around 2000
MB/sec
2. With 100% as schemata both iMC and resctrl mbm_values are around 10000
MB/sec
A 300 MB/sec difference between resctrl_mbm and imc_mbm values is
acceptable at 100% schemata but it isn't acceptable at 10% schemata because
that's a huge difference.
So, fix this by checking for percentage difference instead of absolute
difference i.e. check if the difference between resctrl_mbm value and
imc_mbm value is within 5% (randomly chosen value) of imc_mbm value. If the
difference is greater than 5% of imc_mbm value, treat it is a failure.
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/mba_test.c | 22 +++++++++++++---------
tools/testing/selftests/resctrl/mbm_test.c | 15 ++++++++-------
2 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 3a226effe80c..fd66a831062c 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -12,7 +12,7 @@
#define RESULT_FILE_NAME "result_mba"
#define NUM_OF_RUNS 5
-#define MAX_DIFF 300
+#define MAX_DIFF_PERCENT 5
#define ALLOCATION_MAX 100
#define ALLOCATION_MIN 10
#define ALLOCATION_STEP 10
@@ -62,7 +62,8 @@ static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
allocation++) {
unsigned long avg_bw_imc, avg_bw_resc;
unsigned long sum_bw_imc = 0, sum_bw_resc = 0;
- unsigned long avg_diff;
+ int avg_diff_per;
+ float avg_diff;
/*
* The first run is discarded due to inaccurate value from
@@ -76,16 +77,19 @@ static void show_mba_info(unsigned long *bw_imc, unsigned long *bw_resc)
avg_bw_imc = sum_bw_imc / (NUM_OF_RUNS - 1);
avg_bw_resc = sum_bw_resc / (NUM_OF_RUNS - 1);
- avg_diff = labs((long)(avg_bw_resc - avg_bw_imc));
+ avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
+ avg_diff_per = (int)(avg_diff * 100);
- ksft_print_msg("%s MBA schemata percentage %u smaller than %d %%\n",
- avg_diff > MAX_DIFF ? "fail:" : "pass:",
- ALLOCATION_MAX - ALLOCATION_STEP * allocation,
- MAX_DIFF);
- ksft_print_msg("avg_diff: %lu\n", avg_diff);
+ ksft_print_msg("%s MBA: diff within %d%% for schemata %u\n",
+ avg_diff_per > MAX_DIFF_PERCENT ?
+ "fail:" : "pass:",
+ MAX_DIFF_PERCENT,
+ ALLOCATION_MAX - ALLOCATION_STEP * allocation);
+
+ ksft_print_msg("avg_diff_per: %d%%\n", avg_diff_per);
ksft_print_msg("avg_bw_imc: %lu\n", avg_bw_imc);
ksft_print_msg("avg_bw_resc: %lu\n", avg_bw_resc);
- if (avg_diff > MAX_DIFF)
+ if (avg_diff_per > MAX_DIFF_PERCENT)
failed = true;
}
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 2b4f26013d84..44a89e0267eb 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -11,7 +11,7 @@
#include "resctrl.h"
#define RESULT_FILE_NAME "result_mbm"
-#define MAX_DIFF 300
+#define MAX_DIFF_PERCENT 5
#define NUM_OF_RUNS 5
static int
@@ -19,8 +19,8 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span)
{
unsigned long avg_bw_imc = 0, avg_bw_resc = 0;
unsigned long sum_bw_imc = 0, sum_bw_resc = 0;
- long avg_diff = 0;
- int runs, ret;
+ int runs, ret, avg_diff_per;
+ float avg_diff = 0;
/*
* Discard the first value which is inaccurate due to monitoring setup
@@ -33,12 +33,13 @@ show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, int span)
avg_bw_imc = sum_bw_imc / 4;
avg_bw_resc = sum_bw_resc / 4;
- avg_diff = avg_bw_resc - avg_bw_imc;
+ avg_diff = (float)labs(avg_bw_resc - avg_bw_imc) / avg_bw_imc;
+ avg_diff_per = (int)(avg_diff * 100);
- ret = labs(avg_diff) > MAX_DIFF;
+ ret = avg_diff_per > MAX_DIFF_PERCENT;
ksft_print_msg("%s MBM: diff within %d%%\n",
- ret ? "fail:" : "pass:", MAX_DIFF);
- ksft_print_msg("avg_diff: %lu\n", labs(avg_diff));
+ ret ? "fail:" : "pass:", MAX_DIFF_PERCENT);
+ ksft_print_msg("avg_diff_per: %d%%\n", avg_diff_per);
ksft_print_msg("Span (MB): %d\n", span);
ksft_print_msg("avg_bw_imc: %lu\n", avg_bw_imc);
ksft_print_msg("avg_bw_resc: %lu\n", avg_bw_resc);
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 15/21] selftests/resctrl: Don't hard code value of "no_of_bits" variable
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (13 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 14/21] selftests/resctrl: Fix MBA/MBM results reporting format Fenghua Yu
@ 2021-03-07 14:54 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 16/21] selftests/resctrl: Modularize resctrl test suite main() function Fenghua Yu
` (6 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
Cache related tests (like CAT and CMT) depend on a variable called
no_of_bits to run. no_of_bits defines the number of contiguous bits
that should be set in the CBM mask and a user can pass a value for
no_of_bits using -n command line argument. If a user hasn't passed any
value, it defaults to 5 (randomly chosen value).
Hard coding no_of_bits to 5 will make the cache tests fail to run on
systems that support maximum cbm mask that is less than or equal to 5 bits.
Hence, don't hard code no_of_bits value.
If a user passes a value for "no_of_bits" using -n option, use it.
Otherwise, no_of_bits is equal to half of the maximum number of bits in
the cbm mask.
Please note that CMT test is still hard coded to 5 bits. It will change in
subsequent patches that change CMT test.
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/cat_test.c | 5 ++++-
tools/testing/selftests/resctrl/resctrl_tests.c | 8 ++++++--
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 109363e9a7d7..58f075aa0423 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -130,7 +130,10 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
/* Get max number of bits from default-cabm mask */
count_of_bits = count_bits(long_mask);
- if (n < 1 || n > count_of_bits - 1) {
+ if (!n)
+ n = count_of_bits / 2;
+
+ if (n > count_of_bits - 1) {
ksft_print_msg("Invalid input value for no_of_bits n!\n");
ksft_print_msg("Please Enter value in range 1 to %d\n",
count_of_bits - 1);
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index f1b08afbc3d0..56900738edd6 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -57,7 +57,7 @@ void tests_cleanup(void)
int main(int argc, char **argv)
{
bool has_ben = false, mbm_test = true, mba_test = true, cmt_test = true;
- int res, c, cpu_no = 1, span = 250, argc_new = argc, i, no_of_bits = 5;
+ int res, c, cpu_no = 1, span = 250, argc_new = argc, i, no_of_bits = 0;
char *benchmark_cmd[BENCHMARK_ARGS], bw_report[64], bm_type[64];
char benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE];
int ben_ind, ben_count, tests = 0;
@@ -110,6 +110,10 @@ int main(int argc, char **argv)
break;
case 'n':
no_of_bits = atoi(optarg);
+ if (no_of_bits <= 0) {
+ printf("Bail out! invalid argument for no_of_bits\n");
+ return -1;
+ }
break;
case 'h':
cmd_help();
@@ -188,7 +192,7 @@ int main(int argc, char **argv)
ksft_print_msg("Starting CMT test ...\n");
if (!has_ben)
sprintf(benchmark_cmd[5], "%s", "cmt");
- res = cmt_resctrl_val(cpu_no, no_of_bits, benchmark_cmd);
+ res = cmt_resctrl_val(cpu_no, 5, benchmark_cmd);
ksft_test_result(!res, "CMT: test\n");
cmt_test_cleanup();
}
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 16/21] selftests/resctrl: Modularize resctrl test suite main() function
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (14 preceding siblings ...)
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 ` 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
` (5 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
Resctrl test suite main() function does the following things
1. Parses command line arguments passed by user
2. Some setup checks
3. Logic that calls into each unit test
4. Print result and clean up after running each unit test
Introduce wrapper functions for steps 3 and 4 to modularize the main()
function. Adding these wrapper functions makes it easier to add any logic
to each individual test.
Please note that this is a preparatory patch for the next one and no
functional changes are intended.
Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
.../testing/selftests/resctrl/resctrl_tests.c | 88 ++++++++++++-------
1 file changed, 57 insertions(+), 31 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index 56900738edd6..f9d00ecbeedb 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -54,10 +54,58 @@ void tests_cleanup(void)
cat_test_cleanup();
}
+static void run_mbm_test(bool has_ben, char **benchmark_cmd, int span,
+ int cpu_no, char *bw_report)
+{
+ int res;
+
+ ksft_print_msg("Starting MBM BW change ...\n");
+ if (!has_ben)
+ sprintf(benchmark_cmd[5], "%s", MBA_STR);
+ res = mbm_bw_change(span, cpu_no, bw_report, benchmark_cmd);
+ ksft_test_result(!res, "MBM: bw change\n");
+ mbm_test_cleanup();
+}
+
+static void run_mba_test(bool has_ben, char **benchmark_cmd, int span,
+ int cpu_no, char *bw_report)
+{
+ int res;
+
+ ksft_print_msg("Starting MBA Schemata change ...\n");
+ if (!has_ben)
+ sprintf(benchmark_cmd[1], "%d", span);
+ res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd);
+ ksft_test_result(!res, "MBA: schemata change\n");
+ mba_test_cleanup();
+}
+
+static void run_cmt_test(bool has_ben, char **benchmark_cmd, int cpu_no)
+{
+ int res;
+
+ ksft_print_msg("Starting CMT test ...\n");
+ if (!has_ben)
+ sprintf(benchmark_cmd[5], "%s", "cmt");
+ res = cmt_resctrl_val(cpu_no, 5, benchmark_cmd);
+ ksft_test_result(!res, "CMT: test\n");
+ cmt_test_cleanup();
+}
+
+static void run_cat_test(int cpu_no, int no_of_bits)
+{
+ int res;
+
+ ksft_print_msg("Starting CAT test ...\n");
+ res = cat_perf_miss_val(cpu_no, no_of_bits, "L3");
+ ksft_test_result(!res, "CAT: test\n");
+ cat_test_cleanup();
+}
+
int main(int argc, char **argv)
{
bool has_ben = false, mbm_test = true, mba_test = true, cmt_test = true;
- int res, c, cpu_no = 1, span = 250, argc_new = argc, i, no_of_bits = 0;
+ int c, cpu_no = 1, span = 250, argc_new = argc, i, no_of_bits = 0;
char *benchmark_cmd[BENCHMARK_ARGS], bw_report[64], bm_type[64];
char benchmark_cmd_area[BENCHMARK_ARGS][BENCHMARK_ARG_SIZE];
int ben_ind, ben_count, tests = 0;
@@ -170,39 +218,17 @@ int main(int argc, char **argv)
ksft_set_plan(tests ? : 4);
- if (!is_amd && mbm_test) {
- ksft_print_msg("Starting MBM BW change ...\n");
- if (!has_ben)
- sprintf(benchmark_cmd[5], "%s", MBA_STR);
- res = mbm_bw_change(span, cpu_no, bw_report, benchmark_cmd);
- ksft_test_result(!res, "MBM: bw change\n");
- mbm_test_cleanup();
- }
+ if (!is_amd && mbm_test)
+ run_mbm_test(has_ben, benchmark_cmd, span, cpu_no, bw_report);
- if (!is_amd && mba_test) {
- ksft_print_msg("Starting MBA Schemata change ...\n");
- if (!has_ben)
- sprintf(benchmark_cmd[1], "%d", span);
- res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd);
- ksft_test_result(!res, "MBA: schemata change\n");
- mba_test_cleanup();
- }
+ if (!is_amd && mba_test)
+ run_mba_test(has_ben, benchmark_cmd, span, cpu_no, bw_report);
- if (cmt_test) {
- ksft_print_msg("Starting CMT test ...\n");
- if (!has_ben)
- sprintf(benchmark_cmd[5], "%s", "cmt");
- res = cmt_resctrl_val(cpu_no, 5, benchmark_cmd);
- ksft_test_result(!res, "CMT: test\n");
- cmt_test_cleanup();
- }
+ if (cmt_test)
+ run_cmt_test(has_ben, benchmark_cmd, cpu_no);
- if (cat_test) {
- ksft_print_msg("Starting CAT test ...\n");
- res = cat_perf_miss_val(cpu_no, no_of_bits, "L3");
- ksft_test_result(!res, "CAT: test\n");
- cat_test_cleanup();
- }
+ if (cat_test)
+ run_cat_test(cpu_no, no_of_bits);
return ksft_exit_pass();
}
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 17/21] selftests/resctrl: Skip the test if requested resctrl feature is not supported
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (15 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 16/21] selftests/resctrl: Modularize resctrl test suite main() function Fenghua Yu
@ 2021-03-07 14:54 ` Fenghua Yu
2021-03-07 14:54 ` [PATCH v5 18/21] selftests/resctrl: Fix unmount resctrl FS Fenghua Yu
` (4 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
There could be two reasons why a resctrl feature might not be enabled on
the platform
1. H/W might not support the feature
2. Even if the H/W supports it, the user might have disabled the feature
through kernel command line arguments
Hence, any resctrl unit test (like cmt, cat, mbm and mba) before starting
the test will first check if the feature is enabled on the platform or not.
If the feature isn't enabled, then the test returns with an error status.
For example, if MBA isn't supported on a platform and if the user tries to
run MBA, the output will look like this
ok mounting resctrl to "/sys/fs/resctrl"
not ok MBA: schemata change
But, not supporting a feature isn't a test failure. So, instead of treating
it as an error, use the SKIP directive of the TAP protocol. With the
change, the output will look as below
ok MBA # SKIP Hardware does not support MBA or MBA is disabled
Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/cat_test.c | 3 ---
tools/testing/selftests/resctrl/mba_test.c | 3 ---
tools/testing/selftests/resctrl/mbm_test.c | 3 ---
.../testing/selftests/resctrl/resctrl_tests.c | 23 +++++++++++++++++++
4 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 58f075aa0423..2e531ef0e7e4 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -111,9 +111,6 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
if (ret)
return ret;
- if (!validate_resctrl_feature_request("cat"))
- return -1;
-
/* Get default cbm mask for L3/L2 cache */
ret = get_cbm_mask(cache_type, cbm_mask);
if (ret)
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index fd66a831062c..205f09235b06 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -158,9 +158,6 @@ int mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd)
remove(RESULT_FILE_NAME);
- if (!validate_resctrl_feature_request("mba"))
- return -1;
-
ret = resctrl_val(benchmark_cmd, ¶m);
if (ret)
return ret;
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 44a89e0267eb..01f72d4e68a6 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -131,9 +131,6 @@ int mbm_bw_change(int span, int cpu_no, char *bw_report, char **benchmark_cmd)
remove(RESULT_FILE_NAME);
- if (!validate_resctrl_feature_request("mbm"))
- return -1;
-
ret = resctrl_val(benchmark_cmd, ¶m);
if (ret)
return ret;
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index f9d00ecbeedb..fa4c5f5075dd 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -60,6 +60,12 @@ static void run_mbm_test(bool has_ben, char **benchmark_cmd, int span,
int res;
ksft_print_msg("Starting MBM BW change ...\n");
+
+ if (!validate_resctrl_feature_request("mbm")) {
+ ksft_test_result_skip("Hardware does not support MBM or MBM is disabled\n");
+ return;
+ }
+
if (!has_ben)
sprintf(benchmark_cmd[5], "%s", MBA_STR);
res = mbm_bw_change(span, cpu_no, bw_report, benchmark_cmd);
@@ -73,6 +79,12 @@ static void run_mba_test(bool has_ben, char **benchmark_cmd, int span,
int res;
ksft_print_msg("Starting MBA Schemata change ...\n");
+
+ if (!validate_resctrl_feature_request("mba")) {
+ ksft_test_result_skip("Hardware does not support MBA or MBA is disabled\n");
+ return;
+ }
+
if (!has_ben)
sprintf(benchmark_cmd[1], "%d", span);
res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd);
@@ -85,6 +97,11 @@ static void run_cmt_test(bool has_ben, char **benchmark_cmd, int cpu_no)
int res;
ksft_print_msg("Starting CMT test ...\n");
+ if (!validate_resctrl_feature_request("cmt")) {
+ ksft_test_result_skip("Hardware does not support CMT or CMT is disabled\n");
+ return;
+ }
+
if (!has_ben)
sprintf(benchmark_cmd[5], "%s", "cmt");
res = cmt_resctrl_val(cpu_no, 5, benchmark_cmd);
@@ -97,6 +114,12 @@ static void run_cat_test(int cpu_no, int no_of_bits)
int res;
ksft_print_msg("Starting CAT test ...\n");
+
+ if (!validate_resctrl_feature_request("cat")) {
+ ksft_test_result_skip("Hardware does not support CAT or CAT is disabled\n");
+ return;
+ }
+
res = cat_perf_miss_val(cpu_no, no_of_bits, "L3");
ksft_test_result(!res, "CAT: test\n");
cat_test_cleanup();
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 18/21] selftests/resctrl: Fix unmount resctrl FS
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (16 preceding siblings ...)
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 ` Fenghua Yu
2021-03-07 14:55 ` [PATCH v5 19/21] selftests/resctrl: Fix incorrect parsing of iMC counters Fenghua Yu
` (3 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:54 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
umount_resctrlfs() directly attempts to unmount resctrl file system without
checking if resctrl FS is already mounted or not. It returns 0 on success
and on failure it prints an error message and returns an error status.
Calling umount_resctrlfs() when resctrl FS isn't mounted will return an
error status.
There could be situations where-in the caller might not know if resctrl
FS is already mounted or not and the caller might still want to unmount
resctrl FS if it's already mounted (For example during teardown).
To support above use cases, change umount_resctrlfs() such that it now
first checks if resctrl FS is already mounted or not and unmounts resctrl
FS only if it's already mounted.
unmount resctrl FS upon exit. For example, running only mba test on a
Broadwell (BDW) machine (MBA isn't supported on BDW CPU).
This happens because validate_resctrl_feature_request() would mount resctrl
FS to check if mba is enabled on the platform or not and finds that the H/W
doesn't support mba and hence will return false to run_mba_test(). This in
turn makes the main() function return without unmounting resctrl FS.
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/resctrl_tests.c | 2 ++
tools/testing/selftests/resctrl/resctrlfs.c | 3 +++
2 files changed, 5 insertions(+)
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index fa4c5f5075dd..6204ede25ad1 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -253,5 +253,7 @@ int main(int argc, char **argv)
if (cat_test)
run_cat_test(cpu_no, no_of_bits);
+ umount_resctrlfs();
+
return ksft_exit_pass();
}
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 0c23514760dd..cb52d0ad4be2 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -82,6 +82,9 @@ int remount_resctrlfs(bool mum_resctrlfs)
int umount_resctrlfs(void)
{
+ if (find_resctrl_mount(NULL))
+ return 0;
+
if (umount(RESCTRL_PATH)) {
perror("# Unable to umount resctrl");
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 19/21] selftests/resctrl: Fix incorrect parsing of iMC counters
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (17 preceding siblings ...)
2021-03-07 14:54 ` [PATCH v5 18/21] selftests/resctrl: Fix unmount resctrl FS Fenghua Yu
@ 2021-03-07 14:55 ` Fenghua Yu
2021-03-07 14:55 ` [PATCH v5 20/21] selftests/resctrl: Fix checking for < 0 for unsigned values Fenghua Yu
` (2 subsequent siblings)
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:55 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
iMC (Integrated Memory Controller) counters are usually at
"/sys/bus/event_source/devices/" and are named as "uncore_imc_<n>".
num_of_imcs() function tries to count number of such iMC counters so that
it could appropriately initialize required number of perf_attr structures
that could be used to read these iMC counters.
num_of_imcs() function assumes that all the directories under this path
that start with "uncore_imc" are iMC counters. But, on some systems there
could be directories named as "uncore_imc_free_running" which aren't iMC
counters. Trying to read from such directories will result in "not found
file" errors and MBM/MBA tests will fail.
Hence, fix the logic in num_of_imcs() such that it looks at the first
character after "uncore_imc_" to check if it's a numerical digit or not. If
it's a digit then the directory represents an iMC counter, else, skip the
directory.
Reported-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
tools/testing/selftests/resctrl/resctrl_val.c | 22 +++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 48bcd5fd7d79..de99d398ebfb 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -221,8 +221,8 @@ static int read_from_imc_dir(char *imc_dir, int count)
*/
static int num_of_imcs(void)
{
+ char imc_dir[512], *temp;
unsigned int count = 0;
- char imc_dir[512];
struct dirent *ep;
int ret;
DIR *dp;
@@ -230,7 +230,25 @@ static int num_of_imcs(void)
dp = opendir(DYN_PMU_PATH);
if (dp) {
while ((ep = readdir(dp))) {
- if (strstr(ep->d_name, UNCORE_IMC)) {
+ temp = strstr(ep->d_name, UNCORE_IMC);
+ if (!temp)
+ continue;
+
+ /*
+ * imc counters are named as "uncore_imc_<n>", hence
+ * increment the pointer to point to <n>. Note that
+ * sizeof(UNCORE_IMC) would count for null character as
+ * well and hence the last underscore character in
+ * uncore_imc'_' need not be counted.
+ */
+ temp = temp + sizeof(UNCORE_IMC);
+
+ /*
+ * Some directories under "DYN_PMU_PATH" could have
+ * names like "uncore_imc_free_running", hence, check if
+ * first character is a numerical digit or not.
+ */
+ if (temp[0] >= '0' && temp[0] <= '9') {
sprintf(imc_dir, "%s/%s/", DYN_PMU_PATH,
ep->d_name);
ret = read_from_imc_dir(imc_dir, count);
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 20/21] selftests/resctrl: Fix checking for < 0 for unsigned values
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (18 preceding siblings ...)
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
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
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:55 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu, Dan Carpenter
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
^ permalink raw reply related [flat|nested] 32+ messages in thread* [PATCH v5 21/21] selftests/resctrl: Create .gitignore to include resctrl_tests
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (19 preceding siblings ...)
2021-03-07 14:55 ` [PATCH v5 20/21] selftests/resctrl: Fix checking for < 0 for unsigned values Fenghua Yu
@ 2021-03-07 14:55 ` Fenghua Yu
2021-03-12 19:08 ` [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Babu Moger
21 siblings, 0 replies; 32+ messages in thread
From: Fenghua Yu @ 2021-03-07 14:55 UTC (permalink / raw)
To: Shuah Khan, Tony Luck, Reinette Chatre, Babu Moger
Cc: linux-kselftest, linux-kernel, Fenghua Yu
Create .gitignore to hold the test file resctrl_tests generated after
compiling.
Suggested-by: Shuah Khan <shuah@kernel.org>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
Change Log:
v5:
- Add this patch (Shuah)
tools/testing/selftests/resctrl/.gitignore | 2 ++
1 file changed, 2 insertions(+)
create mode 100644 tools/testing/selftests/resctrl/.gitignore
diff --git a/tools/testing/selftests/resctrl/.gitignore b/tools/testing/selftests/resctrl/.gitignore
new file mode 100644
index 000000000000..ab68442b6bc8
--- /dev/null
+++ b/tools/testing/selftests/resctrl/.gitignore
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+resctrl_tests
--
2.30.1
^ permalink raw reply related [flat|nested] 32+ messages in thread* RE: [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests
2021-03-07 14:54 [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests Fenghua Yu
` (20 preceding siblings ...)
2021-03-07 14:55 ` [PATCH v5 21/21] selftests/resctrl: Create .gitignore to include resctrl_tests Fenghua Yu
@ 2021-03-12 19:08 ` Babu Moger
2021-03-12 22:11 ` Fenghua Yu
21 siblings, 1 reply; 32+ messages in thread
From: Babu Moger @ 2021-03-12 19:08 UTC (permalink / raw)
To: Fenghua Yu, Shuah Khan, Tony Luck, Reinette Chatre
Cc: linux-kselftest, linux-kernel
Hi Fenghua, Thanks for the patches.
Sanity tested them on AMD systems. Appears to work fine.
Few minor comments in few patches.
Tested-by: Babu Moger <babu.moger@amd.com>
Thanks
Babu
> -----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 00/21] Miscellaneous fixes for resctrl selftests
>
> This patch set has several miscellaneous fixes to resctrl selftest tool that are
> easily visible to user. V1 had fixes to CAT test and CMT test but they were
> dropped in V2 because having them here made the patchset humongous. So,
> changes to CAT test and CMT test will be posted in another patchset.
>
> Change Log:
> v5:
> - Address various comments from Shuah Khan:
> 1. Move a few fixing patches before cleaning patches.
> 2. Call kselftest APIs to log test results instead of printf().
> 3. Add .gitignore to ignore resctrl_tests.
> 4. Share show_cache_info() in CAT and CMT tests.
> 5. Define long_mask, cbm_mask, count_of_bits etc as static variables.
>
> v4:
> - Address various comments from Shuah Khan:
> 1. Combine a few patches e.g. a couple of fixing typos patches into one
> and a couple of unmounting patches into one etc.
> 2. Add config file.
> 3. Remove "Fixes" tags.
> 4. Change strcmp() to strncmp().
> 5. Move the global variable fixing patch to the patch 1 so that the
> compilation issue is fixed first.
>
> Please note:
> - I didn't move the patch of renaming CQM to CMT to the end of the series
> because code and commit messages in a few other patches depend on the
> new term of "CMT". If move the renaming patch to the end, the previous
> patches use the old "CQM" term and code which will be changed soon at
> the end of series and will cause more code and explanations.
> [v3:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org
> %2Flkml%2F2020%2F10%2F28%2F137&data=04%7C01%7Cbabu.moger%4
> 0amd.com%7Cd188e85e961f4d246fb208d8e17901e7%7C3dd8961fe4884e608e
> 11a82d994e183d%7C0%7C0%7C637507257122356726%7CUnknown%7CTWFpb
> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6M
> n0%3D%7C1000&sdata=nHYN3OHv3eCZyC29T4cuiyD8SbGKoISOxsXFJHV0
> M54%3D&reserved=0]
>
> v3:
> Address various comments (commit messages, return value on test failure, print
> failure info on test failure etc) from Reinette and Tony.
> [v2:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Flinux-
> kselftest%2Fcover.1589835155.git.sai.praneeth.prakhya%40intel.com%2F&
> ;data=04%7C01%7Cbabu.moger%40amd.com%7Cd188e85e961f4d246fb208d8e
> 17901e7%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6375072571
> 22356726%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=gHzm10%2B76r
> NybPHxD2Uf6OmQINMyptbsH2mXU0QYxfw%3D&reserved=0]
>
> v2:
> 1. Dropped changes to CAT test and CMT test as they will be posted in a later
> series.
> 2. Added several other fixes
> [v1:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ker
> nel.org%2Flinux-
> kselftest%2Fcover.1583657204.git.sai.praneeth.prakhya%40intel.com%2F&
> ;data=04%7C01%7Cbabu.moger%40amd.com%7Cd188e85e961f4d246fb208d8e
> 17901e7%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6375072571
> 22356726%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=vQYNiVeuWVK
> v6Eo8BlE%2BhBSScWbI2bznPmbnAm6esXM%3D&reserved=0]
>
> Fenghua Yu (19):
> selftests/resctrl: Enable gcc checks to detect buffer overflows
> selftests/resctrl: Fix compilation issues for global variables
> selftests/resctrl: Fix compilation issues for other global variables
> selftests/resctrl: Clean up resctrl features check
> selftests/resctrl: Fix missing options "-n" and "-p"
> selftests/resctrl: Rename CQM test as CMT test
> selftests/resctrl: Call kselftest APIs to log test results
> selftests/resctrl: Share show_cache_info() by CAT and CMT tests
> selftests/resctrl: Add config dependencies
> selftests/resctrl: Check for resctrl mount point only if resctrl FS is
> supported
> selftests/resctrl: Use resctrl/info for feature detection
> selftests/resctrl: Fix MBA/MBM results reporting format
> selftests/resctrl: Don't hard code value of "no_of_bits" variable
> selftests/resctrl: Modularize resctrl test suite main() function
> selftests/resctrl: Skip the test if requested resctrl feature is not
> supported
> selftests/resctrl: Fix unmount resctrl FS
> selftests/resctrl: Fix incorrect parsing of iMC counters
> selftests/resctrl: Fix checking for < 0 for unsigned values
> selftests/resctrl: Create .gitignore to include resctrl_tests
>
> Reinette Chatre (2):
> selftests/resctrl: Ensure sibling CPU is not same as original CPU
> selftests/resctrl: Fix a printed message
>
> tools/testing/selftests/resctrl/.gitignore | 2 +
> tools/testing/selftests/resctrl/Makefile | 2 +-
> tools/testing/selftests/resctrl/README | 4 +-
> tools/testing/selftests/resctrl/cache.c | 52 +++++-
> tools/testing/selftests/resctrl/cat_test.c | 57 ++----
> .../resctrl/{cqm_test.c => cmt_test.c} | 75 +++-----
> tools/testing/selftests/resctrl/config | 2 +
> tools/testing/selftests/resctrl/fill_buf.c | 4 +-
> tools/testing/selftests/resctrl/mba_test.c | 43 ++---
> tools/testing/selftests/resctrl/mbm_test.c | 42 ++---
> tools/testing/selftests/resctrl/resctrl.h | 29 +++-
> .../testing/selftests/resctrl/resctrl_tests.c | 163 ++++++++++++------
> tools/testing/selftests/resctrl/resctrl_val.c | 95 ++++++----
> tools/testing/selftests/resctrl/resctrlfs.c | 134 ++++++++------
> 14 files changed, 408 insertions(+), 296 deletions(-) create mode 100644
> tools/testing/selftests/resctrl/.gitignore
> rename tools/testing/selftests/resctrl/{cqm_test.c => cmt_test.c} (56%) create
> mode 100644 tools/testing/selftests/resctrl/config
>
> --
> 2.30.1
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests
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
0 siblings, 1 reply; 32+ messages in thread
From: Fenghua Yu @ 2021-03-12 22:11 UTC (permalink / raw)
To: Babu Moger
Cc: Shuah Khan, Tony Luck, Reinette Chatre, linux-kselftest, linux-kernel
Hi, Babu,
On Fri, Mar 12, 2021 at 01:08:11PM -0600, Babu Moger wrote:
> Hi Fenghua, Thanks for the patches.
> Sanity tested them on AMD systems. Appears to work fine.
> Few minor comments in few patches.
> Tested-by: Babu Moger <babu.moger@amd.com>
I will add Tested-by: Babu Moger in the series and address your
comments.
Thank you for your review!
-Fenghua
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v5 00/21] Miscellaneous fixes for resctrl selftests
2021-03-12 22:11 ` Fenghua Yu
@ 2021-04-02 18:10 ` Shuah Khan
0 siblings, 0 replies; 32+ messages in thread
From: Shuah Khan @ 2021-04-02 18:10 UTC (permalink / raw)
To: Fenghua Yu, Babu Moger
Cc: Shuah Khan, Tony Luck, Reinette Chatre, linux-kselftest,
linux-kernel, Shuah Khan
On 3/12/21 3:11 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On Fri, Mar 12, 2021 at 01:08:11PM -0600, Babu Moger wrote:
>> Hi Fenghua, Thanks for the patches.
>> Sanity tested them on AMD systems. Appears to work fine.
>> Few minor comments in few patches.
>> Tested-by: Babu Moger <babu.moger@amd.com>
>
> I will add Tested-by: Babu Moger in the series and address your
> comments.
>
> Thank you for your review!
>
Looks like a few patches in this series needs updates. Do you plan to
send the new revision for consideration for 5.13?
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 32+ messages in thread