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.
- 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?
Or is naming void pointers "buffers" not appropriate?
- return buf;
- return p;
}