selftests/resctrl: Commonize the signal handler register/unregister for all tests
authorShaopeng Tan <tan.shaopeng@jp.fujitsu.com>
Thu, 13 Apr 2023 07:22:58 +0000 (16:22 +0900)
committerShuah Khan <skhan@linuxfoundation.org>
Thu, 13 Apr 2023 17:34:23 +0000 (11:34 -0600)
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>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
tools/testing/selftests/resctrl/cat_test.c
tools/testing/selftests/resctrl/fill_buf.c
tools/testing/selftests/resctrl/resctrl.h
tools/testing/selftests/resctrl/resctrl_val.c

index 0880575840f90eca250f178d859bddf28b7f9efe..fb1443f888c4c90beabca9c93649ce3458c7a7a5 100644 (file)
@@ -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();
index 3cd0b337eae5c7f6e2a0bf85191574e62ce1b58b..341cc93ca84c4b86535c4fb58043ed52af96ae2e 100644 (file)
@@ -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) {
index 9555a6f683f726bb9352ac405d06b6096aeebaab..87e39456dee082166ac96398c792b9729114684e 100644 (file)
@@ -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);
index e632657995c7e729e2aec758ba98d867a896bb26..ab1eab1e7ff63e838416c67aec1e1469a193639d 100644 (file)
@@ -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();