Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
IronPort-SDR: kAYv95AfXj1TF0NFiwp/qvO2X2XGZ0rJ99qXo0mUOPKBPJfHi+ybfLTGIjDZ/7s2p2fkjkzOhV
 xjjWOB4onWgSirX3txJ8sG+8N0lQYy93Na+X+Nqd2m1Ia3p2J0kGiSuiXV4f2ooDNA3WNVZ+3x
 DpF+ajXYdCI4aged/w64pmc2O3/kYuI/Az1LQjk9jBFk+IWONctLpHMGkMqXSTSr2Tm3QIhRAg
 dO/NP0sw6/kWZjdacUvF+2Zzlz4naGM8gKeFuYl4Wz15ilAfRZS2jTNpV6YRBa92T2nm+a/65g
 9NvwW3hQtd6QzOKHEoAvFYGi
From:   Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
To:     Fenghua Yu <fenghua.yu@intel.com>,
        Reinette Chatre <reinette.chatre@intel.com>,
        Shuah Khan <shuah@kernel.org>
Cc:     linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        tan.shaopeng@jp.fujitsu.com
Subject: [PATCH v3 1/5] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received
Date:   Wed, 16 Feb 2022 11:26:37 +0900
Message-Id: <20220216022641.2998318-2-tan.shaopeng@jp.fujitsu.com>
In-Reply-To: <20220216022641.2998318-1-tan.shaopeng@jp.fujitsu.com>
References: <20220216022641.2998318-1-tan.shaopeng@jp.fujitsu.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

In kselftest framework, a sub test is run using the timeout utility
and it will send SIGTERM to the test upon timeout.

In resctrl_tests, a child process is created by fork() to
run benchmark but SIGTERM is not set in sigaction().
If SIGTERM signal is received, the parent process will be killed,
but the child process still exists.

kill child process before parent process terminates
if SIGTERM signal is received.

Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
---
Some important feedbacks from v1&v2 are addressed as follows:

- Change the order so that current patch 3/3 becomes 1/3. Since without
  the SIGTERM fix, the test would hang if run from the kselftest framework.
  => I changed the order and the SIGTERM fix now becomes patch [1/5].

- Describe that the test is run using the timeout utility and
  it will send SIGTERM to the test upon timeout.
  => I updated the changelog to include this information.

- Describe changes in imperative mood, and address this in all patches.
  See Documentation/process/submitting-patches.rst for more details.
  => I described all my patches' changelog in imperative mood and
     deleted "This commit".

- +	    sigaction(SIGTERM, &sigact, NULL) ||
  This snippet is preceded with a comment that describes its usage
  you could also update it with the expanded use of the kselftest framework.
  => I don't think it is necessary to add other comments.
     Since the current comment already states "Register CTRL-C handler for parent,
     as it has to kill benchmark before exiting", So, when SIGTERM comes,
     the benchmark(child process) should be killed before parent process terminates,
     but it was missing.

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