On 2024-02-22 at 12:12:44 +0200, Ilpo Järvinen wrote:
On Thu, 22 Feb 2024, Maciej Wieczor-Retman wrote:
Ctrl-c handler isn't aware of what test is currently running. Because of that it executes all cleanups even if they aren't necessary. Since the ctrl-c handler uses the sa_sigaction system no parameters can be passed to it as function arguments.
Add a global variable to make ctrl-c handler aware of the currently run test and only execute the correct cleanup callback.
Signed-off-by: Maciej Wieczor-Retman maciej.wieczor-retman@intel.com
Changelog v2:
- Remove tests_cleanup() from resctrl.h.
- Make current_test a const pointer only inside resctrl_val.c. (Ilpo)
tools/testing/selftests/resctrl/resctrl.h | 3 +-- tools/testing/selftests/resctrl/resctrl_tests.c | 14 +++----------- tools/testing/selftests/resctrl/resctrl_val.c | 6 ++++-- 3 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h index 0f49df4961ea..826783b29c9d 100644 --- a/tools/testing/selftests/resctrl/resctrl.h +++ b/tools/testing/selftests/resctrl/resctrl.h @@ -153,7 +153,6 @@ int resctrl_val(const struct resctrl_test *test, const struct user_params *uparams, const char * const *benchmark_cmd, struct resctrl_val_param *param); -void tests_cleanup(void); void mbm_test_cleanup(void); void mba_test_cleanup(void); unsigned long create_bit_mask(unsigned int start, unsigned int len); @@ -162,7 +161,7 @@ int get_full_cbm(const char *cache_type, unsigned long *mask); int get_mask_no_shareable(const char *cache_type, unsigned long *mask); int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size); void ctrlc_handler(int signum, siginfo_t *info, void *ptr); -int signal_handler_register(void); +int signal_handler_register(const struct resctrl_test *test); void signal_handler_unregister(void); void cat_test_cleanup(void); unsigned int count_bits(unsigned long n); diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index 75fc49ba3efb..161f5365b4f0 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -75,19 +75,11 @@ static void cmd_help(void) printf("\t-h: help\n"); } -void tests_cleanup(void) -{
- mbm_test_cleanup();
- mba_test_cleanup();
- cmt_test_cleanup();
- cat_test_cleanup();
-}
-static int test_prepare(void) +static int test_prepare(const struct resctrl_test *test) { int res;
- res = signal_handler_register();
- res = signal_handler_register(test); if (res) { ksft_print_msg("Failed to register signal handler\n"); return res;
@@ -130,7 +122,7 @@ static void run_single_test(const struct resctrl_test *test, const struct user_p ksft_print_msg("Starting %s test ...\n", test->name);
- if (test_prepare()) {
- if (test_prepare(test)) { ksft_exit_fail_msg("Abnormal failure when preparing for the test\n"); return; }
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c index 5a49f07a6c85..d572815436f3 100644 --- a/tools/testing/selftests/resctrl/resctrl_val.c +++ b/tools/testing/selftests/resctrl/resctrl_val.c @@ -62,6 +62,7 @@ struct imc_counter_config { static char mbm_total_path[1024]; static int imcs; static struct imc_counter_config imc_counters_config[MAX_IMCS][2]; +const struct resctrl_test *current_test;
static const struct
Okay, I'll add it.
void membw_initialize_perf_event_attr(int i, int j) { @@ -472,7 +473,7 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr) if (bm_pid) kill(bm_pid, SIGKILL); umount_resctrlfs();
- tests_cleanup();
- current_test->cleanup();
These calls should have if (current_test->cleanup()) guard. Isn't the non-contiguous already test w/o the cleanup function?
Yes, I remembered to put the check in the main test function but I forgot to put the check here too, thanks.
I'll just resend the corrected version today.
Other than those two, this looked okay.
-- i.