After creating a child process with fork() in CAT test, if a signal such as SIGINT is received, the parent process will be terminated immediately, and therefore the child process will not be killed and also resctrlfs is not unmounted.
There is a signal handler registered in CMT/MBM/MBA tests, which kills child process, unmount resctrlfs, cleanups result files, etc., if a signal such as SIGINT is received.
Commonize the signal handler registered for CMT/MBM/MBA tests and reuse it in CAT.
To reuse the signal handler to kill child process use global bm_pid instead of local bm_pid.
Also, since the MBA/MBA/CMT/CAT are run in order, unregister the signal handler at the end of each test so that the signal handler cannot be inherited by other tests.
Reviewed-by: Ilpo Jarvinen ilpo.jarvinen@linux.intel.com Reviewed-by: Reinette Chatre reinette.chatre@intel.com Signed-off-by: Shaopeng Tan tan.shaopeng@jp.fujitsu.com --- tools/testing/selftests/resctrl/cat_test.c | 9 ++- tools/testing/selftests/resctrl/fill_buf.c | 14 ---- tools/testing/selftests/resctrl/resctrl.h | 2 + tools/testing/selftests/resctrl/resctrl_val.c | 66 ++++++++++++++----- 4 files changed, 58 insertions(+), 33 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 0880575840f9..fb1443f888c4 100644 --- a/tools/testing/selftests/resctrl/cat_test.c +++ b/tools/testing/selftests/resctrl/cat_test.c @@ -103,7 +103,6 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) unsigned long l_mask, l_mask_1; int ret, pipefd[2], sibling_cpu_no; char pipe_message; - pid_t bm_pid;
cache_size = 0;
@@ -181,6 +180,12 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) strcpy(param.filename, RESULT_FILE_NAME1); param.num_of_runs = 0; param.cpu_no = sibling_cpu_no; + } else { + ret = signal_handler_register(); + if (ret) { + kill(bm_pid, SIGKILL); + goto out; + } }
remove(param.filename); @@ -217,8 +222,10 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) } close(pipefd[0]); kill(bm_pid, SIGKILL); + signal_handler_unregister(); }
+out: cat_test_cleanup(); if (bm_pid) umount_resctrlfs(); diff --git a/tools/testing/selftests/resctrl/fill_buf.c b/tools/testing/selftests/resctrl/fill_buf.c index 3cd0b337eae5..341cc93ca84c 100644 --- a/tools/testing/selftests/resctrl/fill_buf.c +++ b/tools/testing/selftests/resctrl/fill_buf.c @@ -32,14 +32,6 @@ static void sb(void) #endif }
-static void ctrl_handler(int signo) -{ - free(startptr); - printf("\nEnding\n"); - sb(); - exit(EXIT_SUCCESS); -} - static void cl_flush(void *p) { #if defined(__i386) || defined(__x86_64) @@ -201,12 +193,6 @@ int run_fill_buf(unsigned long span, int malloc_and_init_memory, unsigned long long cache_size = span; int ret;
- /* set up ctrl-c handler */ - if (signal(SIGINT, ctrl_handler) == SIG_ERR) - printf("Failed to catch SIGINT!\n"); - if (signal(SIGHUP, ctrl_handler) == SIG_ERR) - printf("Failed to catch SIGHUP!\n"); - ret = fill_cache(cache_size, malloc_and_init_memory, memflush, op, resctrl_val); if (ret) { diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 9555a6f683f7..87e39456dee0 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -109,6 +109,8 @@ void mba_test_cleanup(void); int get_cbm_mask(char *cache_type, char *cbm_mask); int get_cache_size(int cpu_no, char *cache_type, unsigned long *cache_size); void ctrlc_handler(int signum, siginfo_t *info, void *ptr); +int signal_handler_register(void); +void signal_handler_unregister(void); int cat_val(struct resctrl_val_param *param); void cat_test_cleanup(void); int cat_perf_miss_val(int cpu_no, int no_of_bits, char *cache_type); diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index e632657995c7..ab1eab1e7ff6 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -476,6 +476,45 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr) exit(EXIT_SUCCESS); }
+/* + * Register CTRL-C handler for parent, as it has to kill + * child process before exiting. + */ +int signal_handler_register(void) +{ + struct sigaction sigact; + int ret = 0; + + sigact.sa_sigaction = ctrlc_handler; + sigemptyset(&sigact.sa_mask); + sigact.sa_flags = SA_SIGINFO; + if (sigaction(SIGINT, &sigact, NULL) || + sigaction(SIGTERM, &sigact, NULL) || + sigaction(SIGHUP, &sigact, NULL)) { + perror("# sigaction"); + ret = -1; + } + return ret; +} + +/* + * Reset signal handler to SIG_DFL. + * Non-Value return because the caller should keep + * the error code of other path even if sigaction fails. + */ +void signal_handler_unregister(void) +{ + struct sigaction sigact; + + sigact.sa_handler = SIG_DFL; + sigemptyset(&sigact.sa_mask); + if (sigaction(SIGINT, &sigact, NULL) || + sigaction(SIGTERM, &sigact, NULL) || + sigaction(SIGHUP, &sigact, NULL)) { + perror("# sigaction"); + } +} + /* * print_results_bw: the memory bandwidth results are stored in a file * @filename: file that stores the results @@ -671,39 +710,28 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
ksft_print_msg("Benchmark PID: %d\n", bm_pid);
- /* - * Register CTRL-C handler for parent, as it has to kill benchmark - * before exiting - */ - sigact.sa_sigaction = ctrlc_handler; - sigemptyset(&sigact.sa_mask); - sigact.sa_flags = SA_SIGINFO; - if (sigaction(SIGINT, &sigact, NULL) || - sigaction(SIGTERM, &sigact, NULL) || - sigaction(SIGHUP, &sigact, NULL)) { - perror("# sigaction"); - ret = errno; + ret = signal_handler_register(); + if (ret) goto out; - }
value.sival_ptr = benchmark_cmd;
/* Taskset benchmark to specified cpu */ ret = taskset_benchmark(bm_pid, param->cpu_no); if (ret) - goto out; + goto unregister;
/* Write benchmark to specified control&monitoring grp in resctrl FS */ ret = write_bm_pid_to_resctrl(bm_pid, param->ctrlgrp, param->mongrp, resctrl_val); if (ret) - goto out; + goto unregister;
if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) || !strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) { ret = initialize_mem_bw_imc(); if (ret) - goto out; + goto unregister;
initialize_mem_bw_resctrl(param->ctrlgrp, param->mongrp, param->cpu_no, resctrl_val); @@ -718,7 +746,7 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param) sizeof(pipe_message)) { perror("# failed reading message from child process"); close(pipefd[0]); - goto out; + goto unregister; } } close(pipefd[0]); @@ -727,7 +755,7 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param) if (sigqueue(bm_pid, SIGUSR1, value) == -1) { perror("# sigqueue SIGUSR1 to child"); ret = errno; - goto out; + goto unregister; }
/* Give benchmark enough time to fully run */ @@ -756,6 +784,8 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param) } }
+unregister: + signal_handler_unregister(); out: kill(bm_pid, SIGKILL); umount_resctrlfs();