[PATCH v2 0/3] selftests/resctrl: Simplify test cleanup functions

[PATCH v2 0/3] selftests/resctrl: Simplify test cleanup functions

[Maciej Wieczor-Retman]

Cleaning up after tests is implemented separately for individual tests
and called at the end of each test execution. Since these functions are
very similar and a more generalized test framework was introduced a
function pointer in the resctrl_test struct can be used to reduce the
amount of function calls.

These functions are also all called in the ctrl-c handler because the
handler isn't aware which test is currently running. Since the handler
is implemented with a sigaction no function parameters can be passed
there but information about what test is currently running can be passed
with a global variable.

Changelog v2:
- Make current_test a const pointer limited in scope to resctrl_val
  file.
- Remove tests_cleanup from resctrl.h.
- Cleanup 'goto out' path and labels in individual test functions.

Older versions of this series:
[v1] https://lore.kernel.org/all/cover.1708434017.git.maciej.wieczor-retman@intel.com/

Maciej Wieczor-Retman (3):
  selftests/resctrl: Add cleanup function to test framework
  selftests/resctrl: Simplify cleanup in ctrl-c handler
  selftests/resctrl: Move cleanups out of individual tests

 tools/testing/selftests/resctrl/cat_test.c      |  8 +++-----
 tools/testing/selftests/resctrl/cmt_test.c      |  4 ++--
 tools/testing/selftests/resctrl/mba_test.c      |  8 +++-----
 tools/testing/selftests/resctrl/mbm_test.c      |  8 +++-----
 tools/testing/selftests/resctrl/resctrl.h       |  9 +++------
 tools/testing/selftests/resctrl/resctrl_tests.c | 16 +++++-----------
 tools/testing/selftests/resctrl/resctrl_val.c   |  6 ++++--
 7 files changed, 23 insertions(+), 36 deletions(-)

-- 
2.43.2

[PATCH v2 1/3] selftests/resctrl: Add cleanup function to test framework

[Maciej Wieczor-Retman]

Resctrl selftests use very similar functions to cleanup after
themselves. This creates a lot of code duplication. Also not being
hooked to the test framework means that ctrl-c handler isn't aware of
what test is currently running and executes all cleanups even though
only one is needed.

Add a function pointer to the resctrl_test struct and attach to it
cleanup functions from individual tests.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
 tools/testing/selftests/resctrl/cat_test.c | 1 +
 tools/testing/selftests/resctrl/cmt_test.c | 1 +
 tools/testing/selftests/resctrl/mba_test.c | 1 +
 tools/testing/selftests/resctrl/mbm_test.c | 1 +
 tools/testing/selftests/resctrl/resctrl.h  | 2 ++
 5 files changed, 6 insertions(+)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 24af8310288a..2d2f69d3e5b7 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -299,4 +299,5 @@ struct resctrl_test l3_cat_test = {
 	.resource = "L3",
 	.feature_check = test_resource_feature_check,
 	.run_test = cat_run_test,
+	.cleanup = cat_test_cleanup,
 };
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index dd5ca343c469..32ddee87e43d 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -178,4 +178,5 @@ struct resctrl_test cmt_test = {
 	.resource = "L3",
 	.feature_check = cmt_feature_check,
 	.run_test = cmt_run_test,
+	.cleanup = cmt_test_cleanup,
 };
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index da256d2dbe5c..7cc4067ce930 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -180,4 +180,5 @@ struct resctrl_test mba_test = {
 	.vendor_specific = ARCH_INTEL,
 	.feature_check = mba_feature_check,
 	.run_test = mba_run_test,
+	.cleanup = mba_test_cleanup,
 };
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 34879e7b71a0..071e2d3808a7 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -150,4 +150,5 @@ struct resctrl_test mbm_test = {
 	.vendor_specific = ARCH_INTEL,
 	.feature_check = mbm_feature_check,
 	.run_test = mbm_run_test,
+	.cleanup = mbm_test_cleanup,
 };
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index c52eaf46f24d..0f49df4961ea 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -70,6 +70,7 @@ struct user_params {
  * @disabled:		Test is disabled
  * @feature_check:	Callback to check required resctrl features
  * @run_test:		Callback to run the test
+ * @cleanup:		Callback to cleanup after the test
  */
 struct resctrl_test {
 	const char	*name;
@@ -79,6 +80,7 @@ struct resctrl_test {
 	bool		(*feature_check)(const struct resctrl_test *test);
 	int		(*run_test)(const struct resctrl_test *test,
 				    const struct user_params *uparams);
+	void		(*cleanup)(void);
 };
 
 /*
-- 
2.43.2

[PATCH v2 2/3] selftests/resctrl: Simplify cleanup in ctrl-c handler

[Maciej Wieczor-Retman]

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;
 
 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();
 	ksft_print_msg("Ending\n\n");
 
 	exit(EXIT_SUCCESS);
@@ -482,13 +483,14 @@ void ctrlc_handler(int signum, siginfo_t *info, void *ptr)
  * Register CTRL-C handler for parent, as it has to kill
  * child process before exiting.
  */
-int signal_handler_register(void)
+int signal_handler_register(const struct resctrl_test *test)
 {
 	struct sigaction sigact = {};
 	int ret = 0;
 
 	bm_pid = 0;
 
+	current_test = test;
 	sigact.sa_sigaction = ctrlc_handler;
 	sigemptyset(&sigact.sa_mask);
 	sigact.sa_flags = SA_SIGINFO;
-- 
2.43.2

Re: [PATCH v2 2/3] selftests/resctrl: Simplify cleanup in ctrl-c handler

[Ilpo Järvinen]

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

>  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?

Other than those two, this looked okay.

-- 
 i.

Re: [PATCH v2 2/3] selftests/resctrl: Simplify cleanup in ctrl-c handler

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

On Thu, 22 Feb 2024, 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>
> > ---

> > 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
> 
> >  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 

Err, if (current_test->cleanup) is what I meant.

> non-contiguous already test w/o the cleanup function?
> 
> Other than those two, this looked okay.

-- 
 i.

Re: [PATCH v2 2/3] selftests/resctrl: Simplify cleanup in ctrl-c handler

[Maciej Wieczor-Retman]

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.
>

-- 
Kind regards
Maciej Wieczór-Retman

[PATCH v2 3/3] selftests/resctrl: Move cleanups out of individual tests

[Maciej Wieczor-Retman]

Every test calls its cleanup function at the end of it's test function.
After the cleanup function pointer is added to the test framework this
can be simplified to executing the callback function at the end of the
generic test running function.

Make test cleanup functions static and call them from the end of
run_single_test() from the resctrl_test's cleanup function pointer.

Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v2:
- Change most goto out paths into return ret. (Ilpo)

 tools/testing/selftests/resctrl/cat_test.c      | 7 ++-----
 tools/testing/selftests/resctrl/cmt_test.c      | 3 +--
 tools/testing/selftests/resctrl/mba_test.c      | 7 ++-----
 tools/testing/selftests/resctrl/mbm_test.c      | 7 ++-----
 tools/testing/selftests/resctrl/resctrl.h       | 4 ----
 tools/testing/selftests/resctrl/resctrl_tests.c | 2 ++
 6 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
index 2d2f69d3e5b7..1d1efed6164e 100644
--- a/tools/testing/selftests/resctrl/cat_test.c
+++ b/tools/testing/selftests/resctrl/cat_test.c
@@ -128,7 +128,7 @@ static int check_results(struct resctrl_val_param *param, const char *cache_type
 	return fail;
 }
 
-void cat_test_cleanup(void)
+static void cat_test_cleanup(void)
 {
 	remove(RESULT_FILE_NAME);
 }
@@ -284,13 +284,10 @@ static int cat_run_test(const struct resctrl_test *test, const struct user_param
 
 	ret = cat_test(test, uparams, &param, span, start_mask);
 	if (ret)
-		goto out;
+		return ret;
 
 	ret = check_results(&param, test->resource,
 			    cache_total_size, full_cache_mask, start_mask);
-out:
-	cat_test_cleanup();
-
 	return ret;
 }
 
diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
index 32ddee87e43d..c477f3c9635f 100644
--- a/tools/testing/selftests/resctrl/cmt_test.c
+++ b/tools/testing/selftests/resctrl/cmt_test.c
@@ -91,7 +91,7 @@ static int check_results(struct resctrl_val_param *param, size_t span, int no_of
 				 MAX_DIFF, MAX_DIFF_PERCENT, runs - 1, true);
 }
 
-void cmt_test_cleanup(void)
+static void cmt_test_cleanup(void)
 {
 	remove(RESULT_FILE_NAME);
 }
@@ -161,7 +161,6 @@ static int cmt_run_test(const struct resctrl_test *test, const struct user_param
 		ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n");
 
 out:
-	cmt_test_cleanup();
 	free(span_str);
 
 	return ret;
diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c
index 7cc4067ce930..a2f81d900900 100644
--- a/tools/testing/selftests/resctrl/mba_test.c
+++ b/tools/testing/selftests/resctrl/mba_test.c
@@ -137,7 +137,7 @@ static int check_results(void)
 	return show_mba_info(bw_imc, bw_resc);
 }
 
-void mba_test_cleanup(void)
+static void mba_test_cleanup(void)
 {
 	remove(RESULT_FILE_NAME);
 }
@@ -158,13 +158,10 @@ static int mba_run_test(const struct resctrl_test *test, const struct user_param
 
 	ret = resctrl_val(test, uparams, uparams->benchmark_cmd, &param);
 	if (ret)
-		goto out;
+		return ret;
 
 	ret = check_results();
 
-out:
-	mba_test_cleanup();
-
 	return ret;
 }
 
diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
index 071e2d3808a7..6589154c102e 100644
--- a/tools/testing/selftests/resctrl/mbm_test.c
+++ b/tools/testing/selftests/resctrl/mbm_test.c
@@ -105,7 +105,7 @@ static int mbm_setup(const struct resctrl_test *test,
 	return ret;
 }
 
-void mbm_test_cleanup(void)
+static void mbm_test_cleanup(void)
 {
 	remove(RESULT_FILE_NAME);
 }
@@ -126,15 +126,12 @@ static int mbm_run_test(const struct resctrl_test *test, const struct user_param
 
 	ret = resctrl_val(test, uparams, uparams->benchmark_cmd, &param);
 	if (ret)
-		goto out;
+		return ret;
 
 	ret = check_results(DEFAULT_SPAN);
 	if (ret && (get_vendor() == ARCH_INTEL))
 		ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n");
 
-out:
-	mbm_test_cleanup();
-
 	return ret;
 }
 
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index 826783b29c9d..428ce9174384 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -153,8 +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 mbm_test_cleanup(void);
-void mba_test_cleanup(void);
 unsigned long create_bit_mask(unsigned int start, unsigned int len);
 unsigned int count_contiguous_bits(unsigned long val, unsigned int *start);
 int get_full_cbm(const char *cache_type, unsigned long *mask);
@@ -163,9 +161,7 @@ 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(const struct resctrl_test *test);
 void signal_handler_unregister(void);
-void cat_test_cleanup(void);
 unsigned int count_bits(unsigned long n);
-void cmt_test_cleanup(void);
 
 void perf_event_attr_initialize(struct perf_event_attr *pea, __u64 config);
 void perf_event_initialize_read_format(struct perf_event_read *pe_read);
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index 161f5365b4f0..bae08d1221ec 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -134,6 +134,8 @@ static void run_single_test(const struct resctrl_test *test, const struct user_p
 	}
 
 	ret = test->run_test(test, uparams);
+	if (test->cleanup)
+		test->cleanup();
 	ksft_test_result(!ret, "%s: test\n", test->name);
 
 cleanup:
-- 
2.43.2