mtest01 parent/child process synchronization issue

Hello,

I discovered a problem in mtest01 which causes it to freeze on some occasions.

testcases/kernel/mem/mtest01/mtest01.c +4 -1

 void handler(int signo)
 {
+	if (signo == SIGCHLD)
+		oom_count++;
 	pid_count++;
 }
 

@@ -77,6 +80,7 @@ int main(int argc, char *argv[])
 	act.sa_flags = 0;
 	sigemptyset(&act.sa_mask);
 	sigaction(SIGRTMIN, &act, 0);
+	sigaction(SIGCHLD, &act, 0);
 
 	while ((c = getopt(argc, argv, "c:b:p:wvh")) != -1) {
 		switch (c) {

@@ -268,7 +272,7 @@ int main(int argc, char *argv[])
 
 			while ((((unsigned long long)pre_mem - post_mem) <
 				(unsigned long long)original_maxbytes) &&
-			       pid_count < pid_cntr) {
+			       pid_count < pid_cntr && !oom_count) {
 				sleep(1);
 				sysinfo(&sstats);
 				post_mem =

testcases/kernel/mem/mtest01/mtest01.c +1 -0

+		else if (dowrite)
 			tst_resm(TPASS, "%llu kbytes allocated and used.",
 				 original_maxbytes / 1024);
 		else

Signature · 1 line

2.4.3

Quoted 4 lines

testcases/kernel/mem/mtest01/mtest01.c +0 -0

index 8c9e81c..06f41d5 100644

testcases/kernel/mem/mtest01/mtest01.c line 54

Quoted 5 lines

testcases/kernel/mem/mtest01/mtest01.c +1 -0

@@ -51,9 +51,12 @@
  char *TCID = "mtest01";
  int TST_TOTAL = 1;
  int pid_count = 0;
+int oom_count = 0;

both of these should be static

testcases/kernel/mem/mtest01/mtest01.c int main(int argc, char *argv[]) · line 292

Quoted 7 lines

testcases/kernel/mem/mtest01/mtest01.c +2 -1

@@ -284,7 +288,9 @@ int main(int argc, char *argv[])
  			kill(pid_list[i], SIGKILL);
  			i++;
  		}
-		if (dowrite)
+		if (oom_count)
+			tst_resm(TFAIL, "the child process was killed");

I wouldn't combine two "if"s into one if else block, the information how much was allocated could be useful even when we hit OOM.

But mainly, problem here is that ~6 lines above child processes get killed, so you are racing with SIGCHLD here. I'd suggest to move this TFAIL before kill() loop.

testcases/kernel/mem/mtest01/mtest01.c int main(int argc, char *argv[]) · line 296

Quoted 4 lines

testcases/kernel/mem/mtest01/mtest01.c +1 -0

+		else if (dowrite)
  			tst_resm(TPASS, "%llu kbytes allocated and used.",
  				 original_maxbytes / 1024);
  		else

From: "Jiri Vohanka" <[email protected]>
To: "Jan Stancek" <[email protected]>, [email protected]
Sent: Wednesday, 25 November, 2015 11:47:36 AM
Subject: Re: [LTP] mtest01 parent/child process synchronization issue

Hi,

Quoted 7 lines

Quoted 6 lines

testcases/kernel/mem/mtest01/mtest01.c +0 -0

index 8c9e81c..06f41d5 100644

Quoted 7 lines

testcases/kernel/mem/mtest01/mtest01.c line 54

Quoted 5 lines

testcases/kernel/mem/mtest01/mtest01.c +1 -0

@@ -51,9 +51,12 @@
  char *TCID = "mtest01";
  int TST_TOTAL = 1;
  int pid_count = 0;
+int oom_count = 0;

both of these should be static

You are right, I also added volatile just to be sure.

Quoted 11 lines

testcases/kernel/mem/mtest01/mtest01.c int main(int argc, char *argv[]) · line 292

Quoted 8 lines

testcases/kernel/mem/mtest01/mtest01.c +2 -1

@@ -284,7 +288,9 @@ int main(int argc, char *argv[])
  			kill(pid_list[i], SIGKILL);
  			i++;
  		}
-		if (dowrite)
+		if (oom_count)
+			tst_resm(TFAIL, "the child process was killed");

I wouldn't combine two "if"s into one if else block, the information how much was allocated could be useful even when we hit OOM.

The original_maxbytes tells us how much we want to allocate, not how much was already allocated. You have to watch TINFO messages emitted by child processes to see what was actually allocated. I think that printing ""%llu kbytes allocated" would be misleading since that is true only if the test passes. I kept the if statement at its current place, but I made the fail message more verbose.

Quoted 4 lines

But mainly, problem here is that ~6 lines above child processes get killed, so you are racing with SIGCHLD here. I'd suggest to move this TFAIL before kill() loop.

I moved the part that kills the child processes after PASS/FAIL evaluation to avoid possible race condition.
(Another option would be to uninstall the SIGCHLD signal handler before killing the processes.)

Quoted 4 lines

testcases/kernel/mem/mtest01/mtest01.c int main(int argc, char *argv[]) · line 296

Quoted 4 lines

testcases/kernel/mem/mtest01/mtest01.c +1 -0

+		else if (dowrite)
  			tst_resm(TPASS, "%llu kbytes allocated and used.",
  				 original_maxbytes / 1024);
  		else

I am new to the list and I am not sure what the rules for posting patches are. Should I use 'git send-email', sign the patch or something?

Regards,
Jiri

I am new to the list and I am not sure what the rules for posting patches are. Should I use 'git send-email', sign the patch or something?

From 05227cd214ed7fcb5ba9e5234d06c3866bfe98dc Mon Sep 17 00:00:00 2001
From: jvohanka <[email protected]>
Date: Wed, 18 Nov 2015 14:39:21 +0100
Subject: [PATCH] Watch for SIGCHLD in mtest01 in case the child gets killed by
oom-killer

The mtest01 checks that a specified amount of memory could be allocated. The memory allocation is performed by several child processes. Each child allocates certain amount of memory and then sends SIGRTMIN to the parent. The parent waits until it receives SIGRTMIN from all child processes or until the amount of free memory is decreased by at least the amount it tasked the children to allocate.
The problem is that in certain situations the child process gets killed by oom-killer. The parent never receives SIGRTMIN from that process and there is not sufficient decrease in the amount of free memory, so the parent keeps waiting indefinitely.
This patch fixes the above issue. It modifies the test in such a way that it also waits for SIGCHLD signal (which is sent when the oom-killer terminates the child process). If SIGCHLD is received the test FAILS.