[v3] selftests/powerpc: Kill child processes on SIGINT

Message ID 1533651339-12816-1-git-send-email-leitao@debian.org
State Accepted
Commit 7c27a26e1ed5a7dd709aa19685d2c98f64e1cf0c
Headers show
Series
  • [v3] selftests/powerpc: Kill child processes on SIGINT
Related show

Checks

Context Check Description
snowpatch_ozlabs/build-ppc32 success Test build-ppc32 on branch next
snowpatch_ozlabs/build-ppc64e success Test build-ppc64e on branch next
snowpatch_ozlabs/build-ppc64be success Test build-ppc64be on branch next
snowpatch_ozlabs/build-ppc64le success Test build-ppc64le on branch next
snowpatch_ozlabs/checkpatch success Test checkpatch on branch next
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied

Commit Message

Breno Leitao Aug. 7, 2018, 2:15 p.m.
There are some powerpc selftests, as tm/tm-unavailable, that run for a long
period (>120 seconds), and if it is interrupted, as pressing CRTL-C
(SIGINT), the foreground process (harness) dies but the child process and
threads continue to execute (with PPID = 1 now) in background.

In this case, you'd think the whole test exited, but there are remaining
threads and processes being executed in background. Sometimes these
zombies processes are doing annoying things, as consuming the whole CPU or
dumping things to STDOUT.

This patch fixes this problem by attaching an empty signal handler to
SIGINT in the harness process. This handler will interrupt (EINTR) the
parent process waitpid() call, letting the code to follow through the
normal flow, which will kill all the processes in the child process group.

This patch also fixes a typo.

Signed-off-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
---
 tools/testing/selftests/powerpc/harness.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Michael Ellerman Aug. 13, 2018, 11:23 a.m. | #1
On Tue, 2018-08-07 at 14:15:39 UTC, Breno Leitao wrote:
> There are some powerpc selftests, as tm/tm-unavailable, that run for a long
> period (>120 seconds), and if it is interrupted, as pressing CRTL-C
> (SIGINT), the foreground process (harness) dies but the child process and
> threads continue to execute (with PPID = 1 now) in background.
> 
> In this case, you'd think the whole test exited, but there are remaining
> threads and processes being executed in background. Sometimes these
> zombies processes are doing annoying things, as consuming the whole CPU or
> dumping things to STDOUT.
> 
> This patch fixes this problem by attaching an empty signal handler to
> SIGINT in the harness process. This handler will interrupt (EINTR) the
> parent process waitpid() call, letting the code to follow through the
> normal flow, which will kill all the processes in the child process group.
> 
> This patch also fixes a typo.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/7c27a26e1ed5a7dd709aa19685d2c9

cheers

Patch

diff --git a/tools/testing/selftests/powerpc/harness.c b/tools/testing/selftests/powerpc/harness.c
index 66d31de60b9a..9d7166dfad1e 100644
--- a/tools/testing/selftests/powerpc/harness.c
+++ b/tools/testing/selftests/powerpc/harness.c
@@ -85,13 +85,13 @@  int run_test(int (test_function)(void), char *name)
 	return status;
 }
 
-static void alarm_handler(int signum)
+static void sig_handler(int signum)
 {
-	/* Jut wake us up from waitpid */
+	/* Just wake us up from waitpid */
 }
 
-static struct sigaction alarm_action = {
-	.sa_handler = alarm_handler,
+static struct sigaction sig_action = {
+	.sa_handler = sig_handler,
 };
 
 void test_harness_set_timeout(uint64_t time)
@@ -106,8 +106,14 @@  int test_harness(int (test_function)(void), char *name)
 	test_start(name);
 	test_set_git_version(GIT_VERSION);
 
-	if (sigaction(SIGALRM, &alarm_action, NULL)) {
-		perror("sigaction");
+	if (sigaction(SIGINT, &sig_action, NULL)) {
+		perror("sigaction (sigint)");
+		test_error(name);
+		return 1;
+	}
+
+	if (sigaction(SIGALRM, &sig_action, NULL)) {
+		perror("sigaction (sigalrm)");
 		test_error(name);
 		return 1;
 	}