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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 7 Feb 2023 04:56:24 +0000
From:   "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To:     'Ilpo Järvinen' 
        <ilpo.jarvinen@...ux.intel.com>
CC:     Fenghua Yu <fenghua.yu@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Shuah Khan <shuah@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: 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@...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