From: "Shaopeng Tan (Fujitsu)" <tan.shaopeng@fujitsu.com>
To: "'Ilpo Järvinen'" <ilpo.jarvinen@linux.intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>,
Reinette Chatre <reinette.chatre@intel.com>,
Shuah Khan <shuah@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
"linux-kselftest@vger.kernel.org"
<linux-kselftest@vger.kernel.org>
Subject: RE: [PATCH v6 4/5] selftests/resctrl: Cleanup properly when an error occurs in CAT test
Date: Wed, 8 Feb 2023 02:42:56 +0000 [thread overview]
Message-ID: <OSZPR01MB63287E0DB7DEF1029305D7D98BD89@OSZPR01MB6328.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <406496b7-877b-d9e7-10e-d7b31da51add@linux.intel.com>
Hi Ilpo,
> On Tue, 7 Feb 2023, Shaopeng Tan (Fujitsu) wrote:
>
> > > 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>
> > > > ---
>
> > > > ret = cat_val(¶m);
> > > > - if (ret)
> > > > - return ret;
> > > > -
> > > > - ret = check_results(¶m);
> > > > - if (ret)
> > > > - return ret;
> > > > + if (ret == 0)
> > > > + ret = check_results(¶m);
> > >
> > > 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?
>
> Yes.
>
> Currently, I see your patch doing (mainly) two things:
> 1) cleaning up the messy signal handler logic
> 2) fixing the early return in case of error from cat_val() or
> check_results()
>
> Both are good changes and both are needed to fully fix things. But (IMHO)
> those are indepedent enough that it would warrant to split this change into two.
Thanks for your advice, I will split it in next version
Best regards,
Shaopeng TAN
next prev parent reply other threads:[~2023-02-08 2:44 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-31 5:46 [PATCH v6 0/5] Some improvements of resctrl selftest Shaopeng Tan
2023-01-31 5:46 ` [PATCH v6 1/5] selftests/resctrl: Fix set up schemata with 100% allocation on first run in MBM test Shaopeng Tan
2023-01-31 5:46 ` [PATCH v6 2/5] selftests/resctrl: Return MBA check result and make it to output message Shaopeng Tan
2023-01-31 5:46 ` [PATCH v6 3/5] selftests/resctrl: Flush stdout file buffer before executing fork() Shaopeng Tan
2023-01-31 5:46 ` [PATCH v6 4/5] selftests/resctrl: Cleanup properly when an error occurs in CAT test Shaopeng Tan
2023-02-03 18:24 ` Reinette Chatre
2023-02-06 11:45 ` Ilpo Järvinen
2023-02-07 4:56 ` Shaopeng Tan (Fujitsu)
2023-02-07 8:50 ` Ilpo Järvinen
2023-02-08 2:42 ` Shaopeng Tan (Fujitsu) [this message]
2023-02-07 14:05 ` Ilpo Järvinen
2023-02-08 2:39 ` Shaopeng Tan (Fujitsu)
2023-02-08 7:54 ` Ilpo Järvinen
2023-01-31 5:46 ` [PATCH v6 5/5] selftests/resctrl: Remove duplicate codes that clear each test result file Shaopeng Tan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=OSZPR01MB63287E0DB7DEF1029305D7D98BD89@OSZPR01MB6328.jpnprd01.prod.outlook.com \
--to=tan.shaopeng@fujitsu.com \
--cc=fenghua.yu@intel.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=reinette.chatre@intel.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).