Skip to contentSkip to conversation

controllers/cgroup_fj: fix longtime wait cgroup_fj_proc.

PATCH New [email protected] 2 participants 4 messages Oct 13, 2016 – Oct 17, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 controllers/cgroup_fj: fix longtime wait cgroup_fj_proc.

From: shuwang <[email protected]>

On some machines, when many cgroup_fj_proc created on the background, killall may failed to find and kill them all as the processes are just created and not ready. And that will cause the ltp testrun wait forever. So changed to use kill -9 instead.

---
 testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
testcases/kernel/controllers/cgroup_fj/cgroup_fj_stress.sh +2 -1
index 698aa49..8c21d59 100755
@@ -107,6 +107,7 @@ attach_task()
     if [ -z "$ppid" ]; then
         cgroup_fj_proc&
         pid=$!
+        pids+="$pid "
     else
         pid="$ppid"
     fi
@@ -148,7 +149,7 @@ case $attach_operation in
 "each" )
     tst_resm TINFO "Attaching task to each subgroup"
     attach_task "$start_path" 0
-    ROD killall -9 "cgroup_fj_proc"
+    ROD kill -9 "$pids"
     # Wait for attached tasks to terminate
     wait
     ;;
Signature · 1 line
2.5.0
#1 lore raw

PATCH 1/1 controllers/cgroup_fj: fix longtime wait cgroup_fj_proc.

Hi!

Quoted 4 lines

On some machines, when many cgroup_fj_proc created on the background, killall may failed to find and kill them all as the processes are just created and not ready. And that will cause the ltp testrun wait forever. So changed to use kill -9 instead.

What is the exact race here? What exactly "just created and not ready" means here?

Signature · 2 lines
Cyril Hrubis
[email protected]
author
#2 lore raw

PATCH 1/1 controllers/cgroup_fj: fix longtime wait cgroup_fj_proc.

----- Original Message -----

Quoted 6 lines

From: "Cyril Hrubis" <[email protected]>
To: [email protected]
Cc: [email protected]
Sent: Thursday, October 13, 2016 10:26:04 PM
Subject: Re: [LTP] [PATCH 1/1] controllers/cgroup_fj: fix longtime wait cgroup_fj_proc.

Quoted 8 lines

Hi!

Quoted 4 lines

On some machines, when many cgroup_fj_proc created on the background, killall may failed to find and kill them all as the processes are just created and not ready. And that will cause the ltp testrun wait forever. So changed to use kill -9 instead.

What is the exact race here? What exactly "just created and not ready" means here?

The case cgroup_fj_stress.sh creates many cgroup subgroups according to $1 (subgroup_num) and $2 (subgroup_depth) parameters, and if $3 attach_operation is 'each', it creates cgroup_fj_proc on the background attached to each subgroup.

The race here is to use 'killall -9 cgroup_fj_proc' right after background processes cgroup_fj_proc were created. And a few cgroup_fj_proc processes may not be killed, still running on the background, stalls the wait command.

reproducer:
for i in seq 10
do
sleep 10000 &
done;
killall -9 sleep;
wait; #stall here

Quoted 5 lines

--
Cyril Hrubis
[email protected]

#3 lore raw

PATCH 1/1 controllers/cgroup_fj: fix longtime wait cgroup_fj_proc.

Hi!

Quoted 9 lines

The case cgroup_fj_stress.sh creates many cgroup subgroups according to $1 (subgroup_num) and $2 (subgroup_depth) parameters, and if $3 attach_operation is 'each', it creates cgroup_fj_proc on the background attached to each subgroup.

The race here is to use 'killall -9 cgroup_fj_proc' right after background processes cgroup_fj_proc were created. And a few cgroup_fj_proc processes may not be killed, still running on the background, stalls the wait command.

Quoted 7 lines

reproducer:
for i in seq 10
do
sleep 10000 &
done;
killall -9 sleep;
wait; #stall here

This reproducer should have been in the commit message. I've managed to hit the problem with this once redirected the output from this script into a file. Possibly printing output into stdout slowed it down enough so that the issue haven't shown.

I was thinking if it's safe to use variable to store the pids, since in the each case we fork fair amount of pids (it tops at ~1000) and there is a limit on the command line argument lenght.

For our case it should suffice, even when counting 10 characters to store pid and number we have string that is ~10000 chars long, that is still ~100x times less than usuall limit on the number of pids.

It may still break if someone really wants to stress a machine with a large amount of memory though. If you pass a large enough parameters to the script, it will run probably for a day or two then may fail to kill the processes because the kill command line was too long. So maybe it would be better to store these into a file, but that may slow down the test significantly, which should be avoided as well.

Signature · 2 lines
Cyril Hrubis
[email protected]