RE: [PATCH v6 4/5] selftests/resctrl: Cleanup properly when an error occurs in CAT test

Hi Ilpo,

On Tue, 31 Jan 2023, Shaopeng Tan wrote:

...

After creating a child process with fork() in CAT test, if an error occurs or a signal such as SIGINT is received, the parent process will be terminated immediately, and therefor 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 too.

To reuse the signal handler, make the child process in CAT wait to be killed by parent process in any case (an error occurred or a signal was received), and when killing 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.

Signed-off-by: Shaopeng Tan tan.shaopeng@jp.fujitsu.com

tools/testing/selftests/resctrl/cat_test.c | 28 ++++---- tools/testing/selftests/resctrl/fill_buf.c | 14 ---- tools/testing/selftests/resctrl/resctrl.h | 2 + tools/testing/selftests/resctrl/resctrl_val.c | 70 +++++++++++++------ 4 files changed, 68 insertions(+), 46 deletions(-)

diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c index 6a8306b0a109..3524fa88e3a4 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,28 +180,31 @@ 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);

...

ret = cat_val(&param);

• if (ret)

• return ret;
  

• ret = check_results(&param);
• if (ret)

• return ret;
  

• if (ret == 0)

• ret = check_results(&param);
  

It would be take this program flow fix out of the signal handler change into a separate change.

Do you mean this fix should be separated into two patches?

To make the child process wait to be killed by parent process in any case(an error occurred or a signal was received), I fixed it like this.

This fix was discussed here. https://lore.kernel.org/lkml/2ab9ca20-c757-7dd8-b770-2b84d171cbfb@intel.com/

Best regards, Shaopeng TAN