This series fixes a few cleanup/error handling problems and cleans up
code.
v2:
- Improved changelogs
- Return NULL directly from malloc_and_init_memory()
- Added patch to convert memalign() to posix_memalign()
- Added patch to correct function comment parameter
- Dropped literal -> define patch for now (likely superceded soon)
Fenghua Yu (1):
selftests/resctrl: Change name from CBM_MASK_PATH to INFO_PATH
Ilpo Järvinen (8):
selftests/resctrl: Return NULL if malloc_and_init_memory() did not
alloc mem
selftests/resctrl: Move ->setup() call outside of test specific
branches
selftests/resctrl: Allow ->setup() to return errors
selftests/resctrl: Check for return value after write_schemata()
selftests/resctrl: Replace obsolete memalign() with posix_memalign()
selftests/resctrl: Change initialize_llc_perf() return type to void
selftests/resctrl: Use remount_resctrlfs() consistently with boolean
selftests/resctrl: Correct get_llc_perf() param in function comment
tools/testing/selftests/resctrl/cache.c | 17 +++++++--------
tools/testing/selftests/resctrl/cat_test.c | 4 ++--
tools/testing/selftests/resctrl/cmt_test.c | 9 ++++----
tools/testing/selftests/resctrl/fill_buf.c | 7 +++++--
tools/testing/selftests/resctrl/mba_test.c | 11 +++++++---
tools/testing/selftests/resctrl/mbm_test.c | 4 ++--
tools/testing/selftests/resctrl/resctrl.h | 6 ++++--
tools/testing/selftests/resctrl/resctrl_val.c | 21 +++++++------------
tools/testing/selftests/resctrl/resctrlfs.c | 2 +-
9 files changed, 41 insertions(+), 40 deletions(-)
--
2.30.2
malloc_and_init_memory() in fill_buf isn't checking if memalign()
successfully allocated memory or not before accessing the memory.
Check the return value of memalign() and return NULL if allocating
aligned memory fails.
Fixes: a2561b12fe39 ("selftests/resctrl: Add built in benchmark")
Co-developed-by: Fenghua Yu <[email protected]>
Signed-off-by: Fenghua Yu <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/fill_buf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
index 56ccbeae0638..c20d0a7ecbe6 100644
--- a/tools/testing/selftests/resctrl/fill_buf.c
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -68,6 +68,8 @@ static void *malloc_and_init_memory(size_t s)
size_t s64;
void *p = memalign(PAGE_SIZE, s);
+ if (!p)
+ return NULL;
p64 = (uint64_t *)p;
s64 = s / sizeof(uint64_t);
--
2.30.2
resctrl_val() function is called only by MBM, MBA, and CMT tests which
means the else branch is never used.
Both test branches call param->setup().
Remove the unused else branch and place the ->setup() call outside of
the test specific branches reducing code duplication.
Co-developed-by: Fenghua Yu <[email protected]>
Signed-off-by: Fenghua Yu <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/resctrl_val.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index b32b96356ec7..787546a52849 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -734,29 +734,22 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
/* Test runs until the callback setup() tells the test to stop. */
while (1) {
+ ret = param->setup(1, param);
+ if (ret) {
+ ret = 0;
+ break;
+ }
+
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;
- break;
- }
-
ret = measure_vals(param, &bw_resc_start);
if (ret)
break;
} else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {
- ret = param->setup(1, param);
- if (ret) {
- ret = 0;
- break;
- }
sleep(1);
ret = measure_cache_vals(param, bm_pid);
if (ret)
break;
- } else {
- break;
}
}
--
2.30.2
resctrl_val() assumes ->setup() always returns either 0 to continue
tests or < 0 in case of the normal termination of tests after x runs.
The latter overlaps with normal error returns.
Define END_OF_TESTS (=1) to differentiate the normal termination of
tests and return errors as negative values. Alter callers of ->setup()
to handle errors properly.
Fixes: 790bf585b0ee ("selftests/resctrl: Add Cache Allocation Technology (CAT) selftest")
Fixes: ecdbb911f22d ("selftests/resctrl: Add MBM test")
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/cache.c | 4 +++-
tools/testing/selftests/resctrl/cat_test.c | 2 +-
tools/testing/selftests/resctrl/cmt_test.c | 2 +-
tools/testing/selftests/resctrl/mba_test.c | 2 +-
tools/testing/selftests/resctrl/mbm_test.c | 2 +-
tools/testing/selftests/resctrl/resctrl.h | 2 ++
tools/testing/selftests/resctrl/resctrl_val.c | 4 +++-
7 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 68ff856d36f0..0485863a169f 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -244,10 +244,12 @@ int cat_val(struct resctrl_val_param *param)
while (1) {
if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) {
ret = param->setup(1, param);
- if (ret) {
+ if (ret == END_OF_TESTS) {
ret = 0;
break;
}
+ if (ret < 0)
+ break;
ret = reset_enable_llc_perf(bm_pid, param->cpu_no);
if (ret)
break;
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 1c5e90c63254..2d3c7c77ab6c 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -40,7 +40,7 @@ static int cat_setup(int num, ...)
/* Run NUM_OF_RUNS times */
if (p->num_of_runs >= NUM_OF_RUNS)
- return -1;
+ return END_OF_TESTS;
if (p->num_of_runs == 0) {
sprintf(schemata, "%lx", p->mask);
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index 8968e36db99d..3b0454e7fc82 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -32,7 +32,7 @@ static int cmt_setup(int num, ...)
/* Run NUM_OF_RUNS times */
if (p->num_of_runs >= NUM_OF_RUNS)
- return -1;
+ return END_OF_TESTS;
p->num_of_runs++;
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 1a1bdb6180cf..f32289ae17ae 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -41,7 +41,7 @@ static int mba_setup(int num, ...)
return 0;
if (allocation < ALLOCATION_MIN || allocation > ALLOCATION_MAX)
- return -1;
+ return END_OF_TESTS;
sprintf(allocation_str, "%d", allocation);
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 8392e5c55ed0..280187628054 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -95,7 +95,7 @@ static int mbm_setup(int num, ...)
/* Run NUM_OF_RUNS times */
if (num_of_runs++ >= NUM_OF_RUNS)
- return -1;
+ return END_OF_TESTS;
va_start(param, num);
p = va_arg(param, struct resctrl_val_param *);
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index f0ded31fb3c7..f44fa2de4d98 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -37,6 +37,8 @@
#define ARCH_INTEL 1
#define ARCH_AMD 2
+#define END_OF_TESTS 1
+
#define PARENT_EXIT(err_msg) \
do { \
perror(err_msg); \
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 787546a52849..00864242d76c 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -735,10 +735,12 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
/* Test runs until the callback setup() tells the test to stop. */
while (1) {
ret = param->setup(1, param);
- if (ret) {
+ if (ret == END_OF_TESTS) {
ret = 0;
break;
}
+ if (ret < 0)
+ break;
if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
--
2.30.2
MBA test case writes schemata but it does not check if the write is
successful or not.
Add the error check and return error properly.
Fixes: 01fee6b4d1f9 ("selftests/resctrl: Add MBA test")
Co-developed-by: Fenghua Yu <[email protected]>
Signed-off-by: Fenghua Yu <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/mba_test.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index f32289ae17ae..97dc98c0c949 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -28,6 +28,7 @@ static int mba_setup(int num, ...)
struct resctrl_val_param *p;
char allocation_str[64];
va_list param;
+ int ret;
va_start(param, num);
p = va_arg(param, struct resctrl_val_param *);
@@ -45,7 +46,11 @@ static int mba_setup(int num, ...)
sprintf(allocation_str, "%d", allocation);
- write_schemata(p->ctrlgrp, allocation_str, p->cpu_no, p->resctrl_val);
+ ret = write_schemata(p->ctrlgrp, allocation_str, p->cpu_no,
+ p->resctrl_val);
+ if (ret < 0)
+ return ret;
+
allocation -= ALLOCATION_STEP;
return 0;
--
2.30.2
initialize_llc_perf() unconditionally returns 0.
initialize_llc_perf() performs only memory initialization, none of
which can fail.
Change the return type from int to void to accurately reflect that its
return value doesn't need to be checked. Remove the error checking from
the only callsite.
Co-developed-by: Fenghua Yu <[email protected]>
Signed-off-by: Fenghua Yu <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/cache.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 0485863a169f..585186c874dc 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -48,7 +48,7 @@ static int perf_event_open_llc_miss(pid_t pid, int cpu_no)
return 0;
}
-static int initialize_llc_perf(void)
+static void initialize_llc_perf(void)
{
memset(&pea_llc_miss, 0, sizeof(struct perf_event_attr));
memset(&rf_cqm, 0, sizeof(struct read_format));
@@ -59,8 +59,6 @@ static int initialize_llc_perf(void)
pea_llc_miss.config = PERF_COUNT_HW_CACHE_MISSES;
rf_cqm.nr = 1;
-
- return 0;
}
static int reset_enable_llc_perf(pid_t pid, int cpu_no)
@@ -234,11 +232,8 @@ int cat_val(struct resctrl_val_param *param)
if (ret)
return ret;
- if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) {
- ret = initialize_llc_perf();
- if (ret)
- return ret;
- }
+ if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)))
+ initialize_llc_perf();
/* Test runs until the callback setup() tells the test to stop. */
while (1) {
--
2.30.2
memalign() is obsolete according to its manpage.
Replace memalign() with posix_memalign() and remove malloc.h include
that was there for memalign().
As a pointer is passed into posix_memalign(), initialize *p to NULL
to silence a warning about the function's return value being used as
uninitialized (which is not valid anyway because the error is properly
checked before p is returned).
Suggested-by: Reinette Chatre <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/fill_buf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
index c20d0a7ecbe6..3cd0b337eae5 100644
--- a/tools/testing/selftests/resctrl/fill_buf.c
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -14,7 +14,6 @@
#include <sys/types.h>
#include <sys/wait.h>
#include <inttypes.h>
-#include <malloc.h>
#include <string.h>
#include "resctrl.h"
@@ -64,11 +63,13 @@ static void mem_flush(void *p, size_t s)
static void *malloc_and_init_memory(size_t s)
{
+ void *p = NULL;
uint64_t *p64;
size_t s64;
+ int ret;
- void *p = memalign(PAGE_SIZE, s);
- if (!p)
+ ret = posix_memalign(&p, PAGE_SIZE, s);
+ if (ret < 0)
return NULL;
p64 = (uint64_t *)p;
--
2.30.2
From: Fenghua Yu <[email protected]>
CBM_MASK_PATH is actually the path to resctrl/info.
Change the macro name to correctly indicate what it represents.
[ ij: Tweaked the changelog. ]
Signed-off-by: Fenghua Yu <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/resctrl.h | 2 +-
tools/testing/selftests/resctrl/resctrlfs.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index f44fa2de4d98..20aaa7c0e784 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -28,7 +28,7 @@
#define MB (1024 * 1024)
#define RESCTRL_PATH "/sys/fs/resctrl"
#define PHYS_ID_PATH "/sys/devices/system/cpu/cpu"
-#define CBM_MASK_PATH "/sys/fs/resctrl/info"
+#define INFO_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"
diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
index 6f543e470ad4..cc6cf49e3129 100644
--- a/tools/testing/selftests/resctrl/resctrlfs.c
+++ b/tools/testing/selftests/resctrl/resctrlfs.c
@@ -210,7 +210,7 @@ int get_cbm_mask(char *cache_type, char *cbm_mask)
if (!cbm_mask)
return -1;
- sprintf(cbm_mask_path, "%s/%s/cbm_mask", CBM_MASK_PATH, cache_type);
+ sprintf(cbm_mask_path, "%s/%s/cbm_mask", INFO_PATH, cache_type);
fp = fopen(cbm_mask_path, "r");
if (!fp) {
--
2.30.2
remount_resctrlfs() accepts a boolean value as an argument. Some tests
pass 0/1 and some tests pass true/false.
Make all the callers of remount_resctrlfs() use true/false so that the
parameter usage is consistent across tests.
Co-developed-by: Fenghua Yu <[email protected]>
Signed-off-by: Fenghua Yu <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/cat_test.c | 2 +-
tools/testing/selftests/resctrl/cmt_test.c | 7 +++----
tools/testing/selftests/resctrl/mba_test.c | 2 +-
tools/testing/selftests/resctrl/mbm_test.c | 2 +-
tools/testing/selftests/resctrl/resctrl.h | 2 +-
5 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 2d3c7c77ab6c..08070d4fa735 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -145,7 +145,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
struct resctrl_val_param param = {
.resctrl_val = CAT_STR,
.cpu_no = cpu_no,
- .mum_resctrlfs = 0,
+ .mum_resctrlfs = false,
.setup = cat_setup,
};
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index 3b0454e7fc82..47cde5c02b7f 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -82,12 +82,11 @@ void cmt_test_cleanup(void)
int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
{
- int ret, mum_resctrlfs;
+ int ret;
cache_size = 0;
- mum_resctrlfs = 1;
- ret = remount_resctrlfs(mum_resctrlfs);
+ ret = remount_resctrlfs(true);
if (ret)
return ret;
@@ -118,7 +117,7 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd)
.ctrlgrp = "c1",
.mongrp = "m1",
.cpu_no = cpu_no,
- .mum_resctrlfs = 0,
+ .mum_resctrlfs = false,
.filename = RESULT_FILE_NAME,
.mask = ~(long_mask << n) & long_mask,
.span = cache_size * n / count_of_bits,
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 97dc98c0c949..7defb32ad0de 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -154,7 +154,7 @@ int mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd)
.ctrlgrp = "c1",
.mongrp = "m1",
.cpu_no = cpu_no,
- .mum_resctrlfs = 1,
+ .mum_resctrlfs = true,
.filename = RESULT_FILE_NAME,
.bw_report = bw_report,
.setup = mba_setup
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 280187628054..c9dfa54af42f 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -122,7 +122,7 @@ int mbm_bw_change(int span, int cpu_no, char *bw_report, char **benchmark_cmd)
.mongrp = "m1",
.span = span,
.cpu_no = cpu_no,
- .mum_resctrlfs = 1,
+ .mum_resctrlfs = true,
.filename = RESULT_FILE_NAME,
.bw_report = bw_report,
.setup = mbm_setup
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 20aaa7c0e784..9555a6f683f7 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -64,7 +64,7 @@ struct resctrl_val_param {
char mongrp[64];
int cpu_no;
unsigned long span;
- int mum_resctrlfs;
+ bool mum_resctrlfs;
char filename[64];
char *bw_report;
unsigned long mask;
--
2.30.2
get_llc_perf() function comment refers to cpu_no parameter that does
not exist.
Correct get_llc_perf() the comment to document llc_perf_miss instead.
Signed-off-by: Ilpo Järvinen <[email protected]>
---
tools/testing/selftests/resctrl/cache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 585186c874dc..8a4fe8693be6 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -77,7 +77,7 @@ static int reset_enable_llc_perf(pid_t pid, int cpu_no)
/*
* get_llc_perf: llc cache miss through perf events
- * @cpu_no: CPU number that the benchmark PID is binded to
+ * @llc_perf_miss: LLC miss counter that is filled on success
*
* Perf events like HW_CACHE_MISSES could be used to validate number of
* cache lines allocated.
--
2.30.2
Hi Ilpo,
On 2/15/2023 5:05 AM, Ilpo Järvinen wrote:
> malloc_and_init_memory() in fill_buf isn't checking if memalign()
> successfully allocated memory or not before accessing the memory.
>
> Check the return value of memalign() and return NULL if allocating
> aligned memory fails.
>
> Fixes: a2561b12fe39 ("selftests/resctrl: Add built in benchmark")
> Co-developed-by: Fenghua Yu <[email protected]>
> Signed-off-by: Fenghua Yu <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you.
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
Hi Ilpo,
On 2/15/2023 5:05 AM, Ilpo Järvinen wrote:
> resctrl_val() function is called only by MBM, MBA, and CMT tests which
Surely not a reason for a resubmit, but just fyi ... using "()" implies
that it is a function so there is no need to add the text "function".
> means the else branch is never used.
>
> Both test branches call param->setup().
>
> Remove the unused else branch and place the ->setup() call outside of
> the test specific branches reducing code duplication.
>
> Co-developed-by: Fenghua Yu <[email protected]>
> Signed-off-by: Fenghua Yu <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
Hi Ilpo,
On 2/15/2023 5:05 AM, Ilpo Järvinen wrote:
> resctrl_val() assumes ->setup() always returns either 0 to continue
> tests or < 0 in case of the normal termination of tests after x runs.
> The latter overlaps with normal error returns.
>
> Define END_OF_TESTS (=1) to differentiate the normal termination of
> tests and return errors as negative values. Alter callers of ->setup()
> to handle errors properly.
>
> Fixes: 790bf585b0ee ("selftests/resctrl: Add Cache Allocation Technology (CAT) selftest")
> Fixes: ecdbb911f22d ("selftests/resctrl: Add MBM test")
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
Hi Ilpo,
On 2/15/2023 5:06 AM, Ilpo Järvinen wrote:
> MBA test case writes schemata but it does not check if the write is
> successful or not.
>
> Add the error check and return error properly.
>
> Fixes: 01fee6b4d1f9 ("selftests/resctrl: Add MBA test")
> Co-developed-by: Fenghua Yu <[email protected]>
> Signed-off-by: Fenghua Yu <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
Hi Ilpo,
On 2/15/2023 5:06 AM, Ilpo Järvinen wrote:
> initialize_llc_perf() unconditionally returns 0.
>
> initialize_llc_perf() performs only memory initialization, none of
> which can fail.
>
> Change the return type from int to void to accurately reflect that its
> return value doesn't need to be checked. Remove the error checking from
> the only callsite.
>
> Co-developed-by: Fenghua Yu <[email protected]>
> Signed-off-by: Fenghua Yu <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
Hi Ilpo,
On 2/15/2023 5:06 AM, Ilpo Järvinen wrote:
> memalign() is obsolete according to its manpage.
>
> Replace memalign() with posix_memalign() and remove malloc.h include
> that was there for memalign().
>
> As a pointer is passed into posix_memalign(), initialize *p to NULL
> to silence a warning about the function's return value being used as
> uninitialized (which is not valid anyway because the error is properly
> checked before p is returned).
>
> Suggested-by: Reinette Chatre <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
Hi Ilpo,
On 2/15/2023 5:06 AM, Ilpo Järvinen wrote:
> get_llc_perf() function comment refers to cpu_no parameter that does
> not exist.
>
> Correct get_llc_perf() the comment to document llc_perf_miss instead.
"Correct the get_llc_perf() comment"? This is so minor and I do not think
a reason to resubmit whole series.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
Hi Ilpo,
On 2/15/2023 5:06 AM, Ilpo Järvinen wrote:
> From: Fenghua Yu <[email protected]>
>
> CBM_MASK_PATH is actually the path to resctrl/info.
>
> Change the macro name to correctly indicate what it represents.
>
> [ ij: Tweaked the changelog. ]
>
> Signed-off-by: Fenghua Yu <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
On Wed, 15 Mar 2023, Reinette Chatre wrote:
> Hi Ilpo,
>
> On 2/15/2023 5:06 AM, Ilpo J?rvinen wrote:
> > get_llc_perf() function comment refers to cpu_no parameter that does
> > not exist.
> >
> > Correct get_llc_perf() the comment to document llc_perf_miss instead.
>
> "Correct the get_llc_perf() comment"?
Yes. No matter how many times I read my own changelogs through, my mind
goes to auto-correction mode and I often fail to spot obvious errors such
as this.
Thanks for reviewing.
--
i.
> This is so minor and I do not think a reason to resubmit whole series.
> > Signed-off-by: Ilpo J?rvinen <[email protected]>
> > ---
>
> Thank you
>
> Reviewed-by: Reinette Chatre <[email protected]>
>
> Reinette
>
Hi Ilpo,
On 2/15/2023 5:06 AM, Ilpo Järvinen wrote:
> remount_resctrlfs() accepts a boolean value as an argument. Some tests
> pass 0/1 and some tests pass true/false.
>
> Make all the callers of remount_resctrlfs() use true/false so that the
> parameter usage is consistent across tests.
>
> Co-developed-by: Fenghua Yu <[email protected]>
> Signed-off-by: Fenghua Yu <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
Thank you
Reviewed-by: Reinette Chatre <[email protected]>
Reinette
Hi Ilpo,
On 2/15/2023 5:05 AM, Ilpo Järvinen wrote:
> This series fixes a few cleanup/error handling problems and cleans up
> code.
>
Thank you very much. These are great cleanups. Looks like I missed
sending one response with the others but at this time all patches
in this series should have my Reviewed-by tag.
If the Kselftest team finds them acceptable I hope that they
can help to route them upstream.
Reinette