Hi Ilpo,
On 10/4/24 7:05 AM, Ilpo Järvinen wrote:
On Thu, 12 Sep 2024, Reinette Chatre wrote:
tools/testing/selftests/resctrl/cmt_test.c | 32 ++---- tools/testing/selftests/resctrl/mba_test.c | 13 ++- tools/testing/selftests/resctrl/mbm_test.c | 22 ++-- tools/testing/selftests/resctrl/resctrl.h | 54 ++++++--- .../testing/selftests/resctrl/resctrl_tests.c | 104 +++++++++++++----- tools/testing/selftests/resctrl/resctrl_val.c | 43 ++++---- 6 files changed, 176 insertions(+), 92 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c index 0c045080d808..a7effe76b419 100644 --- a/tools/testing/selftests/resctrl/cmt_test.c +++ b/tools/testing/selftests/resctrl/cmt_test.c @@ -116,15 +116,13 @@ static void cmt_test_cleanup(void) static int cmt_run_test(const struct resctrl_test *test, const struct user_params *uparams) {
- const char * const *cmd = uparams->benchmark_cmd;
- const char *new_cmd[BENCHMARK_ARGS];
- struct fill_buf_param fill_buf = {0};
Empty initializer is enough to have the fields initialized to zero.
Sure.
...
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 82801245e4c1..98462752cb46 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -43,16 +43,35 @@ #define DEFAULT_SPAN (250 * MB) +/*
- fill_buf_param: "fill_buf" benchmark parameters
- @buf_size: Size (in bytes) of buffer used in benchmark.
"fill_buf" allocates and initializes buffer of
@buf_size. User can change value via command line.
- @memflush: If 0 then the buffer will not be flushed after
allocation and initialization, otherwise the
buffer will be flushed. User can change value via
command line.- */
+struct fill_buf_param {
- size_t buf_size;
- int memflush;
bool?
Or is there a plan to use other values than 0 and 1?
Sure. There is no need to keep the original user provided values. I am not aware of any other planned values.
+static struct fill_buf_param *alloc_fill_buf_param(struct user_params *uparams) +{
- struct fill_buf_param *fill_param = NULL;
- char *endptr = NULL;
- if (!uparams->benchmark_cmd[0] || strcmp(uparams->benchmark_cmd[0], "fill_buf"))
return NULL;- fill_param = malloc(sizeof(*fill_param));
- if (!fill_param)
ksft_exit_skip("Unable to allocate memory for fill_buf parameters.\n");- if (uparams->benchmark_cmd[1]) {
errno = 0;fill_param->buf_size = strtoul(uparams->benchmark_cmd[1], &endptr, 10);if (errno || uparams->benchmark_cmd[1] == endptr) {Should this also check that there is no extra garbage?
Good point. Instead of adding another check I plan to replace existing endptr check with a different test that ensures that "*endptr == '\0'".
static void run_benchmark(int signum, siginfo_t *info, void *ucontext) {
- char **benchmark_cmd;
- int ret, memflush;
- size_t span;
- struct benchmark_info *benchmark_info;
- const struct user_params *uparams;
- struct resctrl_val_param *param; FILE *fp;
- int ret;
- benchmark_cmd = info->si_ptr;
- benchmark_info = info->si_ptr;
I'd just assign this directly while defining the local variable.
Sure.
Thank you.
Reinette