On Fri, 27 Oct 2023, Maciej Wieczór-Retman wrote:
On 2023-10-24 at 12:26:12 +0300, Ilpo Järvinen wrote:
There are unnecessary nested calls in fill_buf.c:
- run_fill_buf() calls fill_cache()
- alloc_buffer() calls malloc_and_init_memory()
Simplify the code flow and remove those unnecessary call levels by moving the called code inside the calling function.
Resolve the difference in run_fill_buf() and fill_cache() parameter name into 'buf_size' which is more descriptive than 'span'.
Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
tools/testing/selftests/resctrl/fill_buf.c | 58 +++++++--------------- tools/testing/selftests/resctrl/resctrl.h | 2 +- 2 files changed, 20 insertions(+), 40 deletions(-)
diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c index f9893edda869..9d0b0bf4b85a 100644 --- a/tools/testing/selftests/resctrl/fill_buf.c +++ b/tools/testing/selftests/resctrl/fill_buf.c @@ -51,29 +51,6 @@ static void mem_flush(unsigned char *buf, size_t buf_size) sb(); }
-static void *malloc_and_init_memory(size_t buf_size) -{
- void *p = NULL;
- uint64_t *p64;
- size_t s64;
- int ret;
- ret = posix_memalign(&p, PAGE_SIZE, buf_size);
- if (ret < 0)
return NULL;- p64 = (uint64_t *)p;
- s64 = buf_size / sizeof(uint64_t);
- while (s64 > 0) {
*p64 = (uint64_t)rand();p64 += (CL_SIZE / sizeof(uint64_t));s64 -= (CL_SIZE / sizeof(uint64_t));- }
- return p;
-}
static int fill_one_span_read(unsigned char *buf, size_t buf_size) { unsigned char *end_ptr = buf + buf_size; @@ -137,20 +114,33 @@ static int fill_cache_write(unsigned char *buf, size_t buf_size, bool once)
static unsigned char *alloc_buffer(size_t buf_size, int memflush) {
- unsigned char *buf;
- void *p = NULL;
Is this initialization doing anything? "p" seems to be either overwritten or in case of an error never accessed.
I'm aware of that but the compiler is too stupid to know that p is initialized if there's no error and spits out a warning so I'll have to keep the unnecessary initialization.
- uint64_t *p64;
- size_t s64;
- int ret;
- buf = malloc_and_init_memory(buf_size);
- if (!buf)
ret = posix_memalign(&p, PAGE_SIZE, buf_size);
if (ret < 0) return NULL;
/* Initialize the buffer */
p64 = (uint64_t *)p;
s64 = buf_size / sizeof(uint64_t);
while (s64 > 0) {
*p64 = (uint64_t)rand();p64 += (CL_SIZE / sizeof(uint64_t));s64 -= (CL_SIZE / sizeof(uint64_t));}
/* Flush the memory before using to avoid "cache hot pages" effect */ if (memflush)
mem_flush(buf, buf_size);
mem_flush(p, buf_size);Wouldn't renaming "p" to "buf" keep this relationship with "buf_size" more explicit?
I'll change it to buf. This patch has a long history which preceeds the change where I made the buffer ptr naming more consistent and I didn't realize I departed here again from the consistent naming until you now pointed it out.