Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

selftests/resctrl: Make benchmark parameter passing robust

The benchmark used during the CMT, MBM, and MBA tests can be provided by
the user via (-b) parameter, if not provided the default "fill_buf"
benchmark is used. The user is additionally able to override
any of the "fill_buf" default parameters when running the tests with
"-b fill_buf <fill_buf parameters>".

The "fill_buf" parameters are managed as an array of strings. Using an
array of strings is complex because it requires transformations to/from
strings at every producer and consumer. This is made worse for the
individual tests where the default benchmark parameters values may not
be appropriate and additional data wrangling is required. For example,
the CMT test duplicates the entire array of strings in order to replace
one of the parameters.

More issues appear when combining the usage of an array of strings with
the use case of user overriding default parameters by specifying
"-b fill_buf <parameters>". This use case is fragile with opportunities
to trigger a SIGSEGV because of opportunities for NULL pointers to exist
in the array of strings. For example, by running below (thus by specifying
"fill_buf" should be used but all parameters are NULL):
$ sudo resctrl_tests -t mbm -b fill_buf

Replace the "array of strings" parameters used for "fill_buf" with
new struct fill_buf_param that contains the "fill_buf" parameters that
can be used directly without transformations to/from strings. Two
instances of struct fill_buf_param may exist at any point in time:
* If the user provides new parameters to "fill_buf", the
user parameter structure (struct user_params) will point to a
fully initialized and immutable struct fill_buf_param
containing the user provided parameters.
* If "fill_buf" is the benchmark that should be used by a test,
then the test parameter structure (struct resctrl_val_param)
will point to a fully initialized struct fill_buf_param. The
latter may contain (a) the user provided parameters verbatim,
(b) user provided parameters adjusted to be appropriate for
the test, or (c) the default parameters for "fill_buf" that
is appropriate for the test if the user did not provide
"fill_buf" parameters nor an alternate benchmark.

The existing behavior of CMT test is to use test defined value for the
buffer size even if the user provides another value via command line.
This behavior is maintained since the test requires that the buffer size
matches the size of the cache allocated, and the amount of cache
allocated can instead be changed by the user with the "-n" command line
parameter.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

authored by

Reinette Chatre and committed by
Shuah Khan
e958c21e 76f8f009

+178 -96
+12 -20
tools/testing/selftests/resctrl/cmt_test.c
··· 116 116 117 117 static int cmt_run_test(const struct resctrl_test *test, const struct user_params *uparams) 118 118 { 119 - const char * const *cmd = uparams->benchmark_cmd; 120 - const char *new_cmd[BENCHMARK_ARGS]; 119 + struct fill_buf_param fill_buf = {}; 121 120 unsigned long cache_total_size = 0; 122 121 int n = uparams->bits ? : 5; 123 122 unsigned long long_mask; 124 - char *span_str = NULL; 125 123 int count_of_bits; 126 124 size_t span; 127 - int ret, i; 125 + int ret; 128 126 129 127 ret = get_full_cbm("L3", &long_mask); 130 128 if (ret) ··· 153 155 154 156 span = cache_portion_size(cache_total_size, param.mask, long_mask); 155 157 156 - if (strcmp(cmd[0], "fill_buf") == 0) { 157 - /* Duplicate the command to be able to replace span in it */ 158 - for (i = 0; uparams->benchmark_cmd[i]; i++) 159 - new_cmd[i] = uparams->benchmark_cmd[i]; 160 - new_cmd[i] = NULL; 161 - 162 - ret = asprintf(&span_str, "%zu", span); 163 - if (ret < 0) 164 - return -1; 165 - new_cmd[1] = span_str; 166 - cmd = new_cmd; 158 + if (uparams->fill_buf) { 159 + fill_buf.buf_size = span; 160 + fill_buf.memflush = uparams->fill_buf->memflush; 161 + param.fill_buf = &fill_buf; 162 + } else if (!uparams->benchmark_cmd[0]) { 163 + fill_buf.buf_size = span; 164 + fill_buf.memflush = true; 165 + param.fill_buf = &fill_buf; 167 166 } 168 167 169 168 remove(RESULT_FILE_NAME); 170 169 171 - ret = resctrl_val(test, uparams, cmd, &param); 170 + ret = resctrl_val(test, uparams, &param); 172 171 if (ret) 173 - goto out; 172 + return ret; 174 173 175 174 ret = check_results(&param, span, n); 176 175 if (ret && (get_vendor() == ARCH_INTEL)) 177 176 ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); 178 - 179 - out: 180 - free(span_str); 181 177 182 178 return ret; 183 179 }
+2 -2
tools/testing/selftests/resctrl/fill_buf.c
··· 102 102 *value_sink = ret; 103 103 } 104 104 105 - unsigned char *alloc_buffer(size_t buf_size, int memflush) 105 + unsigned char *alloc_buffer(size_t buf_size, bool memflush) 106 106 { 107 107 void *buf = NULL; 108 108 uint64_t *p64; ··· 130 130 return buf; 131 131 } 132 132 133 - int run_fill_buf(size_t buf_size, int memflush) 133 + int run_fill_buf(size_t buf_size, bool memflush) 134 134 { 135 135 unsigned char *buf; 136 136
+12 -1
tools/testing/selftests/resctrl/mba_test.c
··· 172 172 .setup = mba_setup, 173 173 .measure = mba_measure, 174 174 }; 175 + struct fill_buf_param fill_buf = {}; 175 176 int ret; 176 177 177 178 remove(RESULT_FILE_NAME); 178 179 179 - ret = resctrl_val(test, uparams, uparams->benchmark_cmd, &param); 180 + if (uparams->fill_buf) { 181 + fill_buf.buf_size = uparams->fill_buf->buf_size; 182 + fill_buf.memflush = uparams->fill_buf->memflush; 183 + param.fill_buf = &fill_buf; 184 + } else if (!uparams->benchmark_cmd[0]) { 185 + fill_buf.buf_size = DEFAULT_SPAN; 186 + fill_buf.memflush = true; 187 + param.fill_buf = &fill_buf; 188 + } 189 + 190 + ret = resctrl_val(test, uparams, &param); 180 191 if (ret) 181 192 return ret; 182 193
+11 -11
tools/testing/selftests/resctrl/mbm_test.c
··· 139 139 .setup = mbm_setup, 140 140 .measure = mbm_measure, 141 141 }; 142 - char *endptr = NULL; 143 - size_t span = 0; 142 + struct fill_buf_param fill_buf = {}; 144 143 int ret; 145 144 146 145 remove(RESULT_FILE_NAME); 147 146 148 - if (uparams->benchmark_cmd[0] && strcmp(uparams->benchmark_cmd[0], "fill_buf") == 0) { 149 - if (uparams->benchmark_cmd[1] && *uparams->benchmark_cmd[1] != '\0') { 150 - errno = 0; 151 - span = strtoul(uparams->benchmark_cmd[1], &endptr, 10); 152 - if (errno || *endptr != '\0') 153 - return -EINVAL; 154 - } 147 + if (uparams->fill_buf) { 148 + fill_buf.buf_size = uparams->fill_buf->buf_size; 149 + fill_buf.memflush = uparams->fill_buf->memflush; 150 + param.fill_buf = &fill_buf; 151 + } else if (!uparams->benchmark_cmd[0]) { 152 + fill_buf.buf_size = DEFAULT_SPAN; 153 + fill_buf.memflush = true; 154 + param.fill_buf = &fill_buf; 155 155 } 156 156 157 - ret = resctrl_val(test, uparams, uparams->benchmark_cmd, &param); 157 + ret = resctrl_val(test, uparams, &param); 158 158 if (ret) 159 159 return ret; 160 160 161 - ret = check_results(span); 161 + ret = check_results(param.fill_buf ? param.fill_buf->buf_size : 0); 162 162 if (ret && (get_vendor() == ARCH_INTEL)) 163 163 ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); 164 164
+43 -16
tools/testing/selftests/resctrl/resctrl.h
··· 44 44 #define DEFAULT_SPAN (250 * MB) 45 45 46 46 /* 47 + * fill_buf_param: "fill_buf" benchmark parameters 48 + * @buf_size: Size (in bytes) of buffer used in benchmark. 49 + * "fill_buf" allocates and initializes buffer of 50 + * @buf_size. User can change value via command line. 51 + * @memflush: If false the buffer will not be flushed after 52 + * allocation and initialization, otherwise the 53 + * buffer will be flushed. User can change value via 54 + * command line (via integers with 0 interpreted as 55 + * false and anything else as true). 56 + */ 57 + struct fill_buf_param { 58 + size_t buf_size; 59 + bool memflush; 60 + }; 61 + 62 + /* 47 63 * user_params: User supplied parameters 48 64 * @cpu: CPU number to which the benchmark will be bound to 49 65 * @bits: Number of bits used for cache allocation size 50 66 * @benchmark_cmd: Benchmark command to run during (some of the) tests 67 + * @fill_buf: Pointer to user provided parameters for "fill_buf", 68 + * NULL if user did not provide parameters and test 69 + * specific defaults should be used. 51 70 */ 52 71 struct user_params { 53 72 int cpu; 54 73 int bits; 55 74 const char *benchmark_cmd[BENCHMARK_ARGS]; 75 + const struct fill_buf_param *fill_buf; 56 76 }; 57 77 58 78 /* ··· 107 87 * @init: Callback function to initialize test environment 108 88 * @setup: Callback function to setup per test run environment 109 89 * @measure: Callback that performs the measurement (a single test) 90 + * @fill_buf: Parameters for default "fill_buf" benchmark. 91 + * Initialized with user provided parameters, possibly 92 + * adapted to be relevant to the test. If user does 93 + * not provide parameters for "fill_buf" nor a 94 + * replacement benchmark then initialized with defaults 95 + * appropriate for test. NULL if user provided 96 + * benchmark. 110 97 */ 111 98 struct resctrl_val_param { 112 - const char *ctrlgrp; 113 - const char *mongrp; 114 - char filename[64]; 115 - unsigned long mask; 116 - int num_of_runs; 117 - int (*init)(const struct resctrl_val_param *param, 118 - int domain_id); 119 - int (*setup)(const struct resctrl_test *test, 120 - const struct user_params *uparams, 121 - struct resctrl_val_param *param); 122 - int (*measure)(const struct user_params *uparams, 123 - struct resctrl_val_param *param, 124 - pid_t bm_pid); 99 + const char *ctrlgrp; 100 + const char *mongrp; 101 + char filename[64]; 102 + unsigned long mask; 103 + int num_of_runs; 104 + int (*init)(const struct resctrl_val_param *param, 105 + int domain_id); 106 + int (*setup)(const struct resctrl_test *test, 107 + const struct user_params *uparams, 108 + struct resctrl_val_param *param); 109 + int (*measure)(const struct user_params *uparams, 110 + struct resctrl_val_param *param, 111 + pid_t bm_pid); 112 + struct fill_buf_param *fill_buf; 125 113 }; 126 114 127 115 struct perf_event_read { ··· 166 138 int write_bm_pid_to_resctrl(pid_t bm_pid, const char *ctrlgrp, const char *mongrp); 167 139 int perf_event_open(struct perf_event_attr *hw_event, pid_t pid, int cpu, 168 140 int group_fd, unsigned long flags); 169 - unsigned char *alloc_buffer(size_t buf_size, int memflush); 141 + unsigned char *alloc_buffer(size_t buf_size, bool memflush); 170 142 void mem_flush(unsigned char *buf, size_t buf_size); 171 143 void fill_cache_read(unsigned char *buf, size_t buf_size, bool once); 172 - int run_fill_buf(size_t buf_size, int memflush); 144 + int run_fill_buf(size_t buf_size, bool memflush); 173 145 int initialize_read_mem_bw_imc(void); 174 146 int measure_read_mem_bw(const struct user_params *uparams, 175 147 struct resctrl_val_param *param, pid_t bm_pid); ··· 177 149 int domain_id); 178 150 int resctrl_val(const struct resctrl_test *test, 179 151 const struct user_params *uparams, 180 - const char * const *benchmark_cmd, 181 152 struct resctrl_val_param *param); 182 153 unsigned long create_bit_mask(unsigned int start, unsigned int len); 183 154 unsigned int count_contiguous_bits(unsigned long val, unsigned int *start);
+79 -24
tools/testing/selftests/resctrl/resctrl_tests.c
··· 148 148 test_cleanup(test); 149 149 } 150 150 151 + /* 152 + * Allocate and initialize a struct fill_buf_param with user provided 153 + * (via "-b fill_buf <fill_buf parameters>") parameters. 154 + * 155 + * Use defaults (that may not be appropriate for all tests) for any 156 + * fill_buf parameters omitted by the user. 157 + * 158 + * Historically it may have been possible for user space to provide 159 + * additional parameters, "operation" ("read" vs "write") in 160 + * benchmark_cmd[3] and "once" (run "once" or until terminated) in 161 + * benchmark_cmd[4]. Changing these parameters have never been 162 + * supported with the default of "read" operation and running until 163 + * terminated built into the tests. Any unsupported values for 164 + * (original) "fill_buf" parameters are treated as failure. 165 + * 166 + * Return: On failure, forcibly exits the test on any parsing failure, 167 + * returns NULL if no parsing needed (user did not actually provide 168 + * "-b fill_buf"). 169 + * On success, returns pointer to newly allocated and fully 170 + * initialized struct fill_buf_param that caller must free. 171 + */ 172 + static struct fill_buf_param *alloc_fill_buf_param(struct user_params *uparams) 173 + { 174 + struct fill_buf_param *fill_param = NULL; 175 + char *endptr = NULL; 176 + 177 + if (!uparams->benchmark_cmd[0] || strcmp(uparams->benchmark_cmd[0], "fill_buf")) 178 + return NULL; 179 + 180 + fill_param = malloc(sizeof(*fill_param)); 181 + if (!fill_param) 182 + ksft_exit_skip("Unable to allocate memory for fill_buf parameters.\n"); 183 + 184 + if (uparams->benchmark_cmd[1] && *uparams->benchmark_cmd[1] != '\0') { 185 + errno = 0; 186 + fill_param->buf_size = strtoul(uparams->benchmark_cmd[1], &endptr, 10); 187 + if (errno || *endptr != '\0') { 188 + free(fill_param); 189 + ksft_exit_skip("Unable to parse benchmark buffer size.\n"); 190 + } 191 + } else { 192 + fill_param->buf_size = DEFAULT_SPAN; 193 + } 194 + 195 + if (uparams->benchmark_cmd[2] && *uparams->benchmark_cmd[2] != '\0') { 196 + errno = 0; 197 + fill_param->memflush = strtol(uparams->benchmark_cmd[2], &endptr, 10) != 0; 198 + if (errno || *endptr != '\0') { 199 + free(fill_param); 200 + ksft_exit_skip("Unable to parse benchmark memflush parameter.\n"); 201 + } 202 + } else { 203 + fill_param->memflush = true; 204 + } 205 + 206 + if (uparams->benchmark_cmd[3] && *uparams->benchmark_cmd[3] != '\0') { 207 + if (strcmp(uparams->benchmark_cmd[3], "0")) { 208 + free(fill_param); 209 + ksft_exit_skip("Only read operations supported.\n"); 210 + } 211 + } 212 + 213 + if (uparams->benchmark_cmd[4] && *uparams->benchmark_cmd[4] != '\0') { 214 + if (strcmp(uparams->benchmark_cmd[4], "false")) { 215 + free(fill_param); 216 + ksft_exit_skip("fill_buf is required to run until termination.\n"); 217 + } 218 + } 219 + 220 + return fill_param; 221 + } 222 + 151 223 static void init_user_params(struct user_params *uparams) 152 224 { 153 225 memset(uparams, 0, sizeof(*uparams)); ··· 230 158 231 159 int main(int argc, char **argv) 232 160 { 161 + struct fill_buf_param *fill_param = NULL; 233 162 int tests = ARRAY_SIZE(resctrl_tests); 234 163 bool test_param_seen = false; 235 164 struct user_params uparams; 236 - char *span_str = NULL; 237 - int ret, c, i; 165 + int c, i; 238 166 239 167 init_user_params(&uparams); 240 168 ··· 311 239 } 312 240 last_arg: 313 241 242 + fill_param = alloc_fill_buf_param(&uparams); 243 + if (fill_param) 244 + uparams.fill_buf = fill_param; 245 + 314 246 ksft_print_header(); 315 247 316 248 /* ··· 333 257 334 258 filter_dmesg(); 335 259 336 - if (!uparams.benchmark_cmd[0]) { 337 - /* If no benchmark is given by "-b" argument, use fill_buf. */ 338 - uparams.benchmark_cmd[0] = "fill_buf"; 339 - ret = asprintf(&span_str, "%u", DEFAULT_SPAN); 340 - if (ret < 0) 341 - ksft_exit_fail_msg("Out of memory!\n"); 342 - uparams.benchmark_cmd[1] = span_str; 343 - uparams.benchmark_cmd[2] = "1"; 344 - /* 345 - * Third parameter was previously used for "operation" 346 - * (read/write) of which only (now default) "read"/"0" 347 - * works. 348 - * Fourth parameter was previously used to indicate 349 - * how long "fill_buf" should run for, with "false" 350 - * ("fill_buf" will keep running until terminated) 351 - * the only option that works. 352 - */ 353 - uparams.benchmark_cmd[3] = NULL; 354 - uparams.benchmark_cmd[4] = NULL; 355 - } 356 - 357 260 ksft_set_plan(tests); 358 261 359 262 for (i = 0; i < ARRAY_SIZE(resctrl_tests); i++) 360 263 run_single_test(resctrl_tests[i], &uparams); 361 264 362 - free(span_str); 265 + free(fill_param); 363 266 ksft_finished(); 364 267 }
+19 -22
tools/testing/selftests/resctrl/resctrl_val.c
··· 535 535 return ret; 536 536 } 537 537 538 + struct benchmark_info { 539 + const struct user_params *uparams; 540 + struct resctrl_val_param *param; 541 + }; 542 + 538 543 /* 539 544 * run_benchmark - Run a specified benchmark or fill_buf (default benchmark) 540 545 * in specified signal. Direct benchmark stdio to /dev/null. ··· 549 544 */ 550 545 static void run_benchmark(int signum, siginfo_t *info, void *ucontext) 551 546 { 552 - char **benchmark_cmd; 553 - int ret, memflush; 554 - size_t span; 547 + struct benchmark_info *benchmark_info = info->si_ptr; 548 + const struct user_params *uparams = benchmark_info->uparams; 549 + struct resctrl_val_param *param = benchmark_info->param; 555 550 FILE *fp; 556 - 557 - benchmark_cmd = info->si_ptr; 551 + int ret; 558 552 559 553 /* 560 554 * Direct stdio of child to /dev/null, so that only parent writes to ··· 565 561 parent_exit(ppid); 566 562 } 567 563 568 - if (strcmp(benchmark_cmd[0], "fill_buf") == 0) { 569 - /* Execute default fill_buf benchmark */ 570 - span = strtoul(benchmark_cmd[1], NULL, 10); 571 - memflush = atoi(benchmark_cmd[2]); 572 - 573 - if (run_fill_buf(span, memflush)) 564 + if (param->fill_buf) { 565 + if (run_fill_buf(param->fill_buf->buf_size, 566 + param->fill_buf->memflush)) 574 567 fprintf(stderr, "Error in running fill buffer\n"); 575 - } else { 568 + } else if (uparams->benchmark_cmd[0]) { 576 569 /* Execute specified benchmark */ 577 - ret = execvp(benchmark_cmd[0], benchmark_cmd); 570 + ret = execvp(uparams->benchmark_cmd[0], (char **)uparams->benchmark_cmd); 578 571 if (ret) 579 572 ksft_perror("execvp"); 580 573 } ··· 586 585 * the benchmark 587 586 * @test: test information structure 588 587 * @uparams: user supplied parameters 589 - * @benchmark_cmd: benchmark command and its arguments 590 588 * @param: parameters passed to resctrl_val() 591 589 * 592 590 * Return: 0 when the test was run, < 0 on error. 593 591 */ 594 592 int resctrl_val(const struct resctrl_test *test, 595 593 const struct user_params *uparams, 596 - const char * const *benchmark_cmd, 597 594 struct resctrl_val_param *param) 598 595 { 596 + struct benchmark_info benchmark_info; 599 597 struct sigaction sigact; 600 598 int ret = 0, pipefd[2]; 601 599 char pipe_message = 0; ··· 609 609 ksft_print_msg("Could not get domain ID\n"); 610 610 return ret; 611 611 } 612 + 613 + benchmark_info.uparams = uparams; 614 + benchmark_info.param = param; 612 615 613 616 /* 614 617 * If benchmark wasn't successfully started by child, then child should ··· 674 671 675 672 ksft_print_msg("Benchmark PID: %d\n", (int)bm_pid); 676 673 677 - /* 678 - * The cast removes constness but nothing mutates benchmark_cmd within 679 - * the context of this process. At the receiving process, it becomes 680 - * argv, which is mutable, on exec() but that's after fork() so it 681 - * doesn't matter for the process running the tests. 682 - */ 683 - value.sival_ptr = (void *)benchmark_cmd; 674 + value.sival_ptr = (void *)&benchmark_info; 684 675 685 676 /* Taskset benchmark to specified cpu */ 686 677 ret = taskset_benchmark(bm_pid, uparams->cpu, NULL);