diff mbox series

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

Message ID 20231024092634.7122-2-ilpo.jarvinen@linux.intel.com
State New
Headers
Series selftests/resctrl: CAT test improvements & generalized test framework |

Commit Message

Ilpo Järvinen Oct. 24, 2023, 9:26 a.m. UTC 
  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 <fenghua.yu@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 tools/testing/selftests/resctrl/fill_buf.c | 26 +++++++++++++---------
 1 file changed, 15 insertions(+), 11 deletions(-)

Comments

Maciej Wieczor-Retman Oct. 27, 2023, 11:24 a.m. UTC | #1 
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 <fenghua.yu@intel.com>
>Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
>Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>---
> 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;
>+
Reinette Chatre Nov. 2, 2023, 5:46 p.m. UTC | #2 
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
diff mbox series

Patch

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)