Re: [PATCH v2 1/5] selftests/resctrl: Kill the child process created by fork() when the SIGTERM signal comes

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Shaopeng Tan, 

On 12/13/2021 2:01 AM, Shaopeng Tan wrote:
> In kselftest framework there is a limited time for each sub test,
> when the time limit comes SIGTEM signal will be sent to sub test by

SIGTEM -> SIGTERM ?

> "timeout --foregroup <seconds>" command.

foregroup?

This is a bit confusing though since it mentions that the "timeout" utility
is called after the test times out. Perhaps you can just describe that, if
present, the test is run using the timeout utility and it will
send SIGTERM to the test upon timeout.

> In resctrl_tests, fork() is used to create a child process.
> This commit ensures child process to be killed before parent process

Especially since I know you are planning more changes in the x86 area
where this is enforced more strictly, please do get into the habit of
describing your changes in imperative mood. Please search for
"This patch" in Documentation/process/submitting-patches.rst (your
usage of "This commit" is equivalent to "This patch") for more details.

Please address this in all your patches where "This commit" is frequently
used. 

> exiting if SIGTERM signal comes.
> 
> Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> ---
>  tools/testing/selftests/resctrl/resctrl_val.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index 95224345c78e..b32b96356ec7 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -678,6 +678,7 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param)
>  	sigemptyset(&sigact.sa_mask);
>  	sigact.sa_flags = SA_SIGINFO;
>  	if (sigaction(SIGINT, &sigact, NULL) ||
> +	    sigaction(SIGTERM, &sigact, NULL) ||
>  	    sigaction(SIGHUP, &sigact, NULL)) {
>  		perror("# sigaction");
>  		ret = errno;

The change looks good. Since this snippet is preceded with a comment
that describes its usage you could also update it with the expanded
use of the kselftest framework.

Reinette