2023-10-24 09:27:49

by Ilpo Järvinen

[permalink] [raw]
Subject: [PATCH 01/24] selftests/resctrl: Split fill_buf to allow tests finer-grained control

MBM, MBA and CMT test cases use run_fill_buf() to loop indefinitely
around the buffer. CAT test case is different and doesn't want to loop
around the buffer continuously.

Split fill_cache() so that both the use cases are easier to control by
creating separate functions for buffer allocation and looping around
the buffer. Make those functions available for tests. The new interface
is based on returning/passing pointers instead of the startptr global
pointer variable that can now be removed. The deallocation can use
free() directly.

This change is part of preparation for new CAT test which allocates a
buffer and does multiple passes over the same buffer (but not in an
infinite loop).

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 | 26 +++++++++++++---------
1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
index 0d425f26583a..f9893edda869 100644
--- a/tools/testing/selftests/resctrl/fill_buf.c
+++ b/tools/testing/selftests/resctrl/fill_buf.c
@@ -135,33 +135,37 @@ static int fill_cache_write(unsigned char *buf, size_t buf_size, bool once)
return 0;
}

-static int fill_cache(size_t buf_size, int memflush, int op, bool once)
+static unsigned char *alloc_buffer(size_t buf_size, int memflush)
{
unsigned char *buf;
- int ret;

buf = malloc_and_init_memory(buf_size);
if (!buf)
- return -1;
+ return NULL;

/* Flush the memory before using to avoid "cache hot pages" effect */
if (memflush)
mem_flush(buf, buf_size);

+ return buf;
+}
+
+static int fill_cache(size_t buf_size, int memflush, int op, bool once)
+{
+ unsigned char *buf;
+ int ret;
+
+ buf = alloc_buffer(buf_size, memflush);
+ if (buf == NULL)
+ return -1;
+
if (op == 0)
ret = fill_cache_read(buf, buf_size, once);
else
ret = fill_cache_write(buf, buf_size, once);
-
free(buf);

- if (ret) {
- printf("\n Error in fill cache read/write...\n");
- return -1;
- }
-
-
- return 0;
+ return ret;
}

int run_fill_buf(size_t span, int memflush, int op, bool once)
--
2.30.2


2023-10-27 11:25:03

by Maciej Wieczor-Retman

[permalink] [raw]
Subject: Re: [PATCH 01/24] selftests/resctrl: Split fill_buf to allow tests finer-grained control

On 2023-10-24 at 12:26:11 +0300, Ilpo J?rvinen wrote:
>MBM, MBA and CMT test cases use run_fill_buf() to loop indefinitely
>around the buffer. CAT test case is different and doesn't want to loop
>around the buffer continuously.
>
>Split fill_cache() so that both the use cases are easier to control by
>creating separate functions for buffer allocation and looping around
>the buffer. Make those functions available for tests. The new interface
>is based on returning/passing pointers instead of the startptr global
>pointer variable that can now be removed. The deallocation can use
>free() directly.
>
>This change is part of preparation for new CAT test which allocates a
>buffer and does multiple passes over the same buffer (but not in an
>infinite loop).
>
>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 | 26 +++++++++++++---------
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
>diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c
>index 0d425f26583a..f9893edda869 100644
>--- a/tools/testing/selftests/resctrl/fill_buf.c
>+++ b/tools/testing/selftests/resctrl/fill_buf.c
>@@ -135,33 +135,37 @@ static int fill_cache_write(unsigned char *buf, size_t buf_size, bool once)
> return 0;
> }
>
>-static int fill_cache(size_t buf_size, int memflush, int op, bool once)
>+static unsigned char *alloc_buffer(size_t buf_size, int memflush)
> {
> unsigned char *buf;
>- int ret;
>
> buf = malloc_and_init_memory(buf_size);
> if (!buf)
>- return -1;
>+ return NULL;
>
> /* Flush the memory before using to avoid "cache hot pages" effect */
> if (memflush)
> mem_flush(buf, buf_size);
>
>+ return buf;
>+}
>+
>+static int fill_cache(size_t buf_size, int memflush, int op, bool once)
>+{
>+ unsigned char *buf;
>+ int ret;
>+
>+ buf = alloc_buffer(buf_size, memflush);
>+ if (buf == NULL)

Maybe just do:
if (!buf)?

Checkpatch also seems to suggest this approach:

CHECK: Comparison to NULL could be written "!buf"
#65: FILE: tools/testing/selftests/resctrl/fill_buf.c:159:
+ if (buf == NULL)

>+ return -1;
>+

--
Kind regards
Maciej Wiecz?r-Retman

2023-11-02 17:46:55

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH 01/24] selftests/resctrl: Split fill_buf to allow tests finer-grained control

Hi Ilpo,

On 10/24/2023 2:26 AM, Ilpo Järvinen wrote:
> MBM, MBA and CMT test cases use run_fill_buf() to loop indefinitely
> around the buffer. CAT test case is different and doesn't want to loop
> around the buffer continuously.

Please do not that the changelog starts by describing issue with
run_fill_buf() (that does not appear in patch) and then switches to
describe solution for fill_cache() below.

>
> Split fill_cache() so that both the use cases are easier to control by
> creating separate functions for buffer allocation and looping around
> the buffer. Make those functions available for tests. The new interface
> is based on returning/passing pointers instead of the startptr global
> pointer variable that can now be removed. The deallocation can use
> free() directly.

Seems like startptr removal has been done already:
5e3e4f1a03f0 ("selftests/resctrl: Remove unnecessary startptr global from fill_buf")

Reinette