Re: Threads stuck in zap_pid_ns_processes()

[Eric W. Biederman]

Guenter Roeck <linux@xxxxxxxxxxxx> writes:

> On Thu, May 11, 2017 at 12:31:21PM -0500, Eric W. Biederman wrote:
>> Guenter Roeck <linux@xxxxxxxxxxxx> writes:
>> 
>> > Hi all,
>> >
>> > the test program attached below almost always results in one of the child
>> > processes being stuck in zap_pid_ns_processes(). When this happens, I can
>> > see from test logs that nr_hashed == 2 and init_pids==1, but there is only
>> > a single thread left in the pid namespace (the one that is stuck).
>> > Traceback from /proc/<pid>/stack is
>> >
>> > [<ffffffff811c385e>] zap_pid_ns_processes+0x1ee/0x2a0
>> > [<ffffffff810c1ba4>] do_exit+0x10d4/0x1330
>> > [<ffffffff810c1ee6>] do_group_exit+0x86/0x130
>> > [<ffffffff810d4347>] get_signal+0x367/0x8a0
>> > [<ffffffff81046e73>] do_signal+0x83/0xb90
>> > [<ffffffff81004475>] exit_to_usermode_loop+0x75/0xc0
>> > [<ffffffff810055b6>] syscall_return_slowpath+0xc6/0xd0
>> > [<ffffffff81ced488>] entry_SYSCALL_64_fastpath+0xab/0xad
>> > [<ffffffffffffffff>] 0xffffffffffffffff
>> >
>> > After 120 seconds, I get the "hung task" message.
>> >
>> > Example from v4.11:
>> >
>> > ...
>> > [ 3263.379545] INFO: task clone:27910 blocked for more than 120 seconds.
>> > [ 3263.379561]       Not tainted 4.11.0+ #1
>> > [ 3263.379569] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> > [ 3263.379577] clone           D    0 27910  27909 0x00000000
>> > [ 3263.379587] Call Trace:
>> > [ 3263.379608]  __schedule+0x677/0xda0
>> > [ 3263.379621]  ? pci_mmcfg_check_reserved+0xc0/0xc0
>> > [ 3263.379634]  ? task_stopped_code+0x70/0x70
>> > [ 3263.379643]  schedule+0x4d/0xd0
>> > [ 3263.379653]  zap_pid_ns_processes+0x1ee/0x2a0
>> > [ 3263.379659]  ? copy_pid_ns+0x4d0/0x4d0
>> > [ 3263.379670]  do_exit+0x10d4/0x1330
>> > ...
>> >
>> > The problem is seen in all kernels up to v4.11.
>> >
>> > Any idea what might be going on and how to fix the problem ?
>> 
>> Let me see.  Reading the code it looks like we have three tasks
>> let's call them main, child1, and child2.
>> 
>> child1 and child2 are started using CLONE_THREAD and are
>> thus clones of one another.
>> 
>> child2 exits first but is ptraced by main so is not reaped.
>>        Further child2 calls do_group_exit forcing child1 to
>>        exit making for fun races.
>> 
>>        A ptread_exit() or syscall(SYS_exit, 0); would skip
>>        the group exit and make the window larger.
>> 
>> child1 exits next and calls zap_pid_ns_processes and is
>>        waiting for child2 to be reaped by main.
>> 
>> main is just sitting around doing nothing for 3600 seconds
>> not reaping anyone.
>> 
>> I would expect that when main exits everything would be cleaned up
>> and the only real issue is that we have a hung task warning.
>> 
>> Does everything cleanup when main exits?
>> 
>
> As an add-on to my previous mail: I added a function to count
> the number of threads in the pid namespace, using next_pidmap().
> Even though nr_hashed == 2, only the hanging thread is still
> present.

For your testcase?  I suspect you copied the code from
zap_pid_ns_processes and skipped pid 1.  It is going to be pid 1 that is
calling zap_pid_ns_processes.

> Is there maybe a better way to terminate the wait loop than
> with "if (pid_ns->nr_hashed == init_pids)" ?

Not right now.  nr_hashed counts the number of "struct pid"s that are
in attached to processes and in the hash table for looking up tasks.

Waiting until that drops to just the last task in the pid namespace
really does do a good job of counting what we are looking for.  It
is a little tricky because if our task_pid(p) != task_tgid(p) then
nr_hashed will be 2.  For various reasons.

That said it isn't as obvious as it should be and I would like
to improve that.

I think the immediate solution here is to just use TASK_INTERRUPTIBLE
instead of TASK_UNITERRUPTIBLE.  This really is not a short term
disk sleep nor anything guaranteed to complete in any sort of
finite time.  Plus the hung task timeout warning only triggers
if your task state is TASK_UNINTERRUPTIBLE so this will prevent
the warning and keep a long delay in zap_pid_ns_processes from
bumping up your load average.

That still leaves the question of what state strange state
you are getting into but this should at least prevent the unexepcted
reboots.

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 77403c157157..971e7bc6939b 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -277,7 +277,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
         * if reparented.
         */
        for (;;) {
-               set_current_state(TASK_UNINTERRUPTIBLE);
+               set_current_state(TASK_INTERRUPTIBLE);
                if (pid_ns->nr_hashed == init_pids)
                        break;
                schedule();


Eric