Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1640001 > unrolled thread

Re: Threads stuck in zap_pid_ns_processes()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Threads stuck in zap_pid_ns_processes() Guenter Roeck <linux@roeck-us.net> - 2017-05-11 22:30 +0200
    Re: Threads stuck in zap_pid_ns_processes() ebiederm@xmission.com (Eric W. Biederman) - 2017-05-11 23:40 +0200
      Re: Threads stuck in zap_pid_ns_processes() Guenter Roeck <linux@roeck-us.net> - 2017-05-12 00:50 +0200
        Re: Threads stuck in zap_pid_ns_processes() ebiederm@xmission.com (Eric W. Biederman) - 2017-05-12 01:30 +0200
          Re: Threads stuck in zap_pid_ns_processes() Vovo Yang <vovoy@google.com> - 2017-05-12 11:40 +0200
            Re: Threads stuck in zap_pid_ns_processes() ebiederm@xmission.com (Eric W. Biederman) - 2017-05-12 15:40 +0200
              Re: Threads stuck in zap_pid_ns_processes() Guenter Roeck <linux@roeck-us.net> - 2017-05-12 19:00 +0200
                Re: Threads stuck in zap_pid_ns_processes() ebiederm@xmission.com (Eric W. Biederman) - 2017-05-12 19:40 +0200
                  [REVIEW][PATCH] pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes ebiederm@xmission.com (Eric W. Biederman) - 2017-05-12 20:10 +0200
                    Re: [REVIEW][PATCH] pid_ns: Sleep in TASK_INTERRUPTIBLE in  zap_pid_ns_processes Guenter Roeck <linux@roeck-us.net> - 2017-05-12 21:40 +0200
                  Re: Threads stuck in zap_pid_ns_processes() Guenter Roeck <linux@roeck-us.net> - 2017-05-12 21:50 +0200
                    Re: Threads stuck in zap_pid_ns_processes() ebiederm@xmission.com (Eric W. Biederman) - 2017-05-12 22:20 +0200
                      Re: Threads stuck in zap_pid_ns_processes() Guenter Roeck <linux@roeck-us.net> - 2017-05-13 16:40 +0200
                        Re: Threads stuck in zap_pid_ns_processes() ebiederm@xmission.com (Eric W. Biederman) - 2017-05-13 20:30 +0200
        Re: Threads stuck in zap_pid_ns_processes() ebiederm@xmission.com (Eric W. Biederman) - 2017-05-12 05:50 +0200

#1640001 — Re: Threads stuck in zap_pid_ns_processes()

On Thu, May 11, 2017 at 12:31:21PM -0500, Eric W. Biederman wrote:
> Guenter Roeck <linux@roeck-us.net> 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.

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

Thanks,
Guenter

[toc] | [next] | [standalone]

#1640058

Guenter Roeck <linux@roeck-us.net> writes:

> On Thu, May 11, 2017 at 12:31:21PM -0500, Eric W. Biederman wrote:
>> Guenter Roeck <linux@roeck-us.net> 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

[toc] | [prev] | [next] | [standalone]

#1640070

On Thu, May 11, 2017 at 04:25:23PM -0500, Eric W. Biederman wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> 
> > On Thu, May 11, 2017 at 12:31:21PM -0500, Eric W. Biederman wrote:
> >> Guenter Roeck <linux@roeck-us.net> 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.
> 

Almost. Something along the line of

	count = 0;
	nr = next_pidmap(pid_ns, 0);
	while (nr > 0) {
		count++;
		nr = next_pidmap(pid_ns, nr);
	}

only I also call sched_show_task() for each thread, and the only
one printed is the one that hangs in 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.
> 

What I know so far is 
- We see this condition on a regular basis in the field. Regular is
  relative, of course - let's say maybe 1 in a Milion Chromebooks
  per day reports a crash because of it. That is not that many,
  but it adds up.
- We are able to reproduce the problem with a performance benchmark
  which opens 100 chrome tabs. While that is a lot, it should not
  result in a kernel hang/crash.
- Vovo proviced the test code last night. I don't know if this is
  exactly what is observed in the benchmark, or how it relates to the
  benchmark in the first place, but it is the first time we are actually
  able to reliably create a condition where the problem is seen.

> 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();
> 

I'll give it a try. Are there any potential unintended side effects ?

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1640072

Guenter Roeck <linux@roeck-us.net> writes:

> On Thu, May 11, 2017 at 04:25:23PM -0500, Eric W. Biederman wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:
>> 
>> > On Thu, May 11, 2017 at 12:31:21PM -0500, Eric W. Biederman wrote:
>> >> Guenter Roeck <linux@roeck-us.net> 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.
>> 
>
> Almost. Something along the line of
>
> 	count = 0;
> 	nr = next_pidmap(pid_ns, 0);
> 	while (nr > 0) {
> 		count++;
> 		nr = next_pidmap(pid_ns, nr);
> 	}
>
> only I also call sched_show_task() for each thread, and the only
> one printed is the one that hangs in 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.
>> 
>
> What I know so far is 
> - We see this condition on a regular basis in the field. Regular is
>   relative, of course - let's say maybe 1 in a Milion Chromebooks
>   per day reports a crash because of it. That is not that many,
>   but it adds up.
> - We are able to reproduce the problem with a performance benchmark
>   which opens 100 chrome tabs. While that is a lot, it should not
>   result in a kernel hang/crash.
> - Vovo proviced the test code last night. I don't know if this is
>   exactly what is observed in the benchmark, or how it relates to the
>   benchmark in the first place, but it is the first time we are actually
>   able to reliably create a condition where the problem is seen.

Thank you.  I will be interesting to hear what is happening in the
chrome perfomance benchmark that triggers this.

There may be some other steps that are worth taking.

>> 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();
>> 
>
> I'll give it a try. Are there any potential unintended side effects ?

No.  The only reason I didn't implement this way in the first place is
that I have cognitive disssonance between the name of that state and
the fact the code isn't interruptible.

In the long term it is probably better to refactor that code path so it
doesn't exist in the first place, and use more common kernel idioms.
But for right now it should not be a problem.


Eric

[toc] | [prev] | [next] | [standalone]

#1640355

On Fri, May 12, 2017 at 7:19 AM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
>
>> On Thu, May 11, 2017 at 04:25:23PM -0500, Eric W. Biederman wrote:
>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>
>>> > On Thu, May 11, 2017 at 12:31:21PM -0500, Eric W. Biederman wrote:
>>> >> Guenter Roeck <linux@roeck-us.net> 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.
>>>
>>
>> Almost. Something along the line of
>>
>>       count = 0;
>>       nr = next_pidmap(pid_ns, 0);
>>       while (nr > 0) {
>>               count++;
>>               nr = next_pidmap(pid_ns, nr);
>>       }
>>
>> only I also call sched_show_task() for each thread, and the only
>> one printed is the one that hangs in 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.
>>>
>>
>> What I know so far is
>> - We see this condition on a regular basis in the field. Regular is
>>   relative, of course - let's say maybe 1 in a Milion Chromebooks
>>   per day reports a crash because of it. That is not that many,
>>   but it adds up.
>> - We are able to reproduce the problem with a performance benchmark
>>   which opens 100 chrome tabs. While that is a lot, it should not
>>   result in a kernel hang/crash.
>> - Vovo proviced the test code last night. I don't know if this is
>>   exactly what is observed in the benchmark, or how it relates to the
>>   benchmark in the first place, but it is the first time we are actually
>>   able to reliably create a condition where the problem is seen.
>
> Thank you.  I will be interesting to hear what is happening in the
> chrome perfomance benchmark that triggers this.
>

What's happening in the benchmark:
1. A chrome renderer process was created with CLONE_NEWPID
2. The process crashed
3. Chrome breakpad service calls ptrace(PTRACE_ATTACH, ..) to attach to every
  threads of the crashed process to dump info
4. When breakpad detach the crashed process, the crashed process stuck in
  zap_pid_ns_processes()

Thanks,
Vovo

> There may be some other steps that are worth taking.
>
>>> 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();
>>>
>>
>> I'll give it a try. Are there any potential unintended side effects ?
>
> No.  The only reason I didn't implement this way in the first place is
> that I have cognitive disssonance between the name of that state and
> the fact the code isn't interruptible.
>
> In the long term it is probably better to refactor that code path so it
> doesn't exist in the first place, and use more common kernel idioms.
> But for right now it should not be a problem.
>
>
> Eric
>

[toc] | [prev] | [next] | [standalone]

#1640445

Vovo Yang <vovoy@google.com> writes:

> On Fri, May 12, 2017 at 7:19 AM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:
>>
>>> What I know so far is
>>> - We see this condition on a regular basis in the field. Regular is
>>>   relative, of course - let's say maybe 1 in a Milion Chromebooks
>>>   per day reports a crash because of it. That is not that many,
>>>   but it adds up.
>>> - We are able to reproduce the problem with a performance benchmark
>>>   which opens 100 chrome tabs. While that is a lot, it should not
>>>   result in a kernel hang/crash.
>>> - Vovo proviced the test code last night. I don't know if this is
>>>   exactly what is observed in the benchmark, or how it relates to the
>>>   benchmark in the first place, but it is the first time we are actually
>>>   able to reliably create a condition where the problem is seen.
>>
>> Thank you.  I will be interesting to hear what is happening in the
>> chrome perfomance benchmark that triggers this.
>>
> What's happening in the benchmark:
> 1. A chrome renderer process was created with CLONE_NEWPID
> 2. The process crashed
> 3. Chrome breakpad service calls ptrace(PTRACE_ATTACH, ..) to attach to every
>   threads of the crashed process to dump info
> 4. When breakpad detach the crashed process, the crashed process stuck in
>   zap_pid_ns_processes()

Very interesting thank you.

So the question is specifically which interaction is causing this.

In the test case provided it was a sibling task in the pid namespace
dying and not being reaped.  Which may be what is happening with
breakpad.  So far I have yet to see kernel bug but I won't rule one out.

Eric

[toc] | [prev] | [next] | [standalone]

#1640609

Hi Eric,

On Fri, May 12, 2017 at 08:26:27AM -0500, Eric W. Biederman wrote:
> Vovo Yang <vovoy@google.com> writes:
> 
> > On Fri, May 12, 2017 at 7:19 AM, Eric W. Biederman
> > <ebiederm@xmission.com> wrote:
> >> Guenter Roeck <linux@roeck-us.net> writes:
> >>
> >>> What I know so far is
> >>> - We see this condition on a regular basis in the field. Regular is
> >>>   relative, of course - let's say maybe 1 in a Milion Chromebooks
> >>>   per day reports a crash because of it. That is not that many,
> >>>   but it adds up.
> >>> - We are able to reproduce the problem with a performance benchmark
> >>>   which opens 100 chrome tabs. While that is a lot, it should not
> >>>   result in a kernel hang/crash.
> >>> - Vovo proviced the test code last night. I don't know if this is
> >>>   exactly what is observed in the benchmark, or how it relates to the
> >>>   benchmark in the first place, but it is the first time we are actually
> >>>   able to reliably create a condition where the problem is seen.
> >>
> >> Thank you.  I will be interesting to hear what is happening in the
> >> chrome perfomance benchmark that triggers this.
> >>
> > What's happening in the benchmark:
> > 1. A chrome renderer process was created with CLONE_NEWPID
> > 2. The process crashed
> > 3. Chrome breakpad service calls ptrace(PTRACE_ATTACH, ..) to attach to every
> >   threads of the crashed process to dump info
> > 4. When breakpad detach the crashed process, the crashed process stuck in
> >   zap_pid_ns_processes()
> 
> Very interesting thank you.
> 
> So the question is specifically which interaction is causing this.
> 
> In the test case provided it was a sibling task in the pid namespace
> dying and not being reaped.  Which may be what is happening with
> breakpad.  So far I have yet to see kernel bug but I won't rule one out.
> 

I am trying to understand what you are looking for. I would have thought
that both the test application as well as the Chrome functionality
described above show that there are situations where zap_pid_ns_processes()
can get stuck and cause hung task timeouts in conjunction with the use of
ptrace().

Your last sentence seems to suggest that you believe that the kernel might
do what it is expected to do. Assuming this is the case, what else would
you like to see ? A test application which matches exactly the Chrome use
case ? We can try to provide that, but I don't entirely understand how
that would change the situation. After all, we already know that it is
possible to get a thread into this condition, and we already have one
means to reproduce it.

Replacing TASK_UNINTERRUPTIBLE with TASK_INTERRUPTABLE works for both the
test application and the Chrome benchmark. The thread is still stuck in
zap_pid_ns_processes(), but it is now in S (sleep) state instead of D,
and no longer results in a hung task timeout. It remains in that state
until the parent process terminates. I am not entirely happy with it
since the processes are still stuck and may pile up over time, but at
least it solves the immediate problem for us.

Question now is what to do with that solution. We can of course apply
it locally to Chrome OS, but I would rather have it upstream - especially
since we have to assume that any users of Chrome on Linux, or more
generically anyone using ptrace in conjunction with CLONE_NEWPID, may
experience the same problem. Right now I have no idea how to get there,
though. Can you provide some guidance ?

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1640661

Guenter Roeck <linux@roeck-us.net> writes:

> Hi Eric,
>
> On Fri, May 12, 2017 at 08:26:27AM -0500, Eric W. Biederman wrote:
>> Vovo Yang <vovoy@google.com> writes:
>> 
>> > On Fri, May 12, 2017 at 7:19 AM, Eric W. Biederman
>> > <ebiederm@xmission.com> wrote:
>> >> Guenter Roeck <linux@roeck-us.net> writes:
>> >>
>> >>> What I know so far is
>> >>> - We see this condition on a regular basis in the field. Regular is
>> >>>   relative, of course - let's say maybe 1 in a Milion Chromebooks
>> >>>   per day reports a crash because of it. That is not that many,
>> >>>   but it adds up.
>> >>> - We are able to reproduce the problem with a performance benchmark
>> >>>   which opens 100 chrome tabs. While that is a lot, it should not
>> >>>   result in a kernel hang/crash.
>> >>> - Vovo proviced the test code last night. I don't know if this is
>> >>>   exactly what is observed in the benchmark, or how it relates to the
>> >>>   benchmark in the first place, but it is the first time we are actually
>> >>>   able to reliably create a condition where the problem is seen.
>> >>
>> >> Thank you.  I will be interesting to hear what is happening in the
>> >> chrome perfomance benchmark that triggers this.
>> >>
>> > What's happening in the benchmark:
>> > 1. A chrome renderer process was created with CLONE_NEWPID
>> > 2. The process crashed
>> > 3. Chrome breakpad service calls ptrace(PTRACE_ATTACH, ..) to attach to every
>> >   threads of the crashed process to dump info
>> > 4. When breakpad detach the crashed process, the crashed process stuck in
>> >   zap_pid_ns_processes()
>> 
>> Very interesting thank you.
>> 
>> So the question is specifically which interaction is causing this.
>> 
>> In the test case provided it was a sibling task in the pid namespace
>> dying and not being reaped.  Which may be what is happening with
>> breakpad.  So far I have yet to see kernel bug but I won't rule one out.
>> 
>
> I am trying to understand what you are looking for. I would have thought
> that both the test application as well as the Chrome functionality
> described above show that there are situations where zap_pid_ns_processes()
> can get stuck and cause hung task timeouts in conjunction with the use of
> ptrace().
>
> Your last sentence seems to suggest that you believe that the kernel might
> do what it is expected to do. Assuming this is the case, what else would
> you like to see ? A test application which matches exactly the Chrome use
> case ? We can try to provide that, but I don't entirely understand how
> that would change the situation. After all, we already know that it is
> possible to get a thread into this condition, and we already have one
> means to reproduce it.
>
> Replacing TASK_UNINTERRUPTIBLE with TASK_INTERRUPTABLE works for both the
> test application and the Chrome benchmark. The thread is still stuck in
> zap_pid_ns_processes(), but it is now in S (sleep) state instead of D,
> and no longer results in a hung task timeout. It remains in that state
> until the parent process terminates. I am not entirely happy with it
> since the processes are still stuck and may pile up over time, but at
> least it solves the immediate problem for us.
>
> Question now is what to do with that solution. We can of course apply
> it locally to Chrome OS, but I would rather have it upstream - especially
> since we have to assume that any users of Chrome on Linux, or more
> generically anyone using ptrace in conjunction with CLONE_NEWPID, may
> experience the same problem. Right now I have no idea how to get there,
> though. Can you provide some guidance ?

Apologies for not being clear.  I intend to send a pull request with the
the TASK_UINTERRUPTIBLE to TASK_INTERRUPTIBLE change to Linus in the
next week or so with a Cc stable and an appropriate Fixes tag.  So the
fix can be backported.

I have a more comprehensive change queued I will probably merge for 4.13
already but it just changes what kind of zombies you see.  It won't
remove the ``stuck'' zombies.

So what I am looking for now is:
Why are things getting stuck in your benchmark?

-  Is it a userspace bug?

  In which case we can figure out what userspace (aka breakpad) needs
   to do to avoid the problem.
   
-  Is it a kernel bug with ptrace?

   There have been a lot of little subtle bugs with ptrace over the
   years so one more would not surprise

So I am just looking to make certain we fix the root issue not just
the hung task timeout warning.

Eric

[toc] | [prev] | [next] | [standalone]

#1640672 — [REVIEW][PATCH] pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes

Date: Thu, 11 May 2017 18:21:01 -0500

The code can potentially sleep for an indefinite amount of time in
zap_pid_ns_processes triggering the hung task timeout, and increasing
the system average.  This is undesirable.  Sleep with a task state of
TASK_INTERRUPTIBLE instead of TASK_UNINTERRUPTIBLE to remove these
undesirable side effects.

Apparently under heavy load this has been allowing Chrome to trigger
the hung time task timeout error and cause ChromeOS to reboot.

Reported-by: Vovo Yang <vovoy@google.com>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Fixes: 6347e9009104 ("pidns: guarantee that the pidns init will be the last pidns process reaped")
Cc: stable@vger.kernel.org
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---

This is what I have queued up posting so it is public knowledge and so
that if anyone can see a flaw it can get fixed.

 kernel/pid_namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index de461aa0bf9a..6e51b8820495 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();
-- 
2.10.1

[toc] | [prev] | [next] | [standalone]

#1640709 — Re: [REVIEW][PATCH] pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes

On Fri, May 12, 2017 at 12:55:22PM -0500, Eric W. Biederman wrote:
> Date: Thu, 11 May 2017 18:21:01 -0500
> 
> The code can potentially sleep for an indefinite amount of time in
> zap_pid_ns_processes triggering the hung task timeout, and increasing
> the system average.  This is undesirable.  Sleep with a task state of
> TASK_INTERRUPTIBLE instead of TASK_UNINTERRUPTIBLE to remove these
> undesirable side effects.
> 
> Apparently under heavy load this has been allowing Chrome to trigger
> the hung time task timeout error and cause ChromeOS to reboot.
> 
> Reported-by: Vovo Yang <vovoy@google.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>

Tested-by: Guenter Roeck <linux@roeck-us.net>

> Fixes: 6347e9009104 ("pidns: guarantee that the pidns init will be the last pidns process reaped")
> Cc: stable@vger.kernel.org
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> 
> This is what I have queued up posting so it is public knowledge and so
> that if anyone can see a flaw it can get fixed.
> 
>  kernel/pid_namespace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index de461aa0bf9a..6e51b8820495 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();
> -- 
> 2.10.1
> 

[toc] | [prev] | [next] | [standalone]

#1640713

Hi Eric,

On Fri, May 12, 2017 at 12:33:01PM -0500, Eric W. Biederman wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> 
> > Hi Eric,
> >
> > On Fri, May 12, 2017 at 08:26:27AM -0500, Eric W. Biederman wrote:
> >> Vovo Yang <vovoy@google.com> writes:
> >> 
> >> > On Fri, May 12, 2017 at 7:19 AM, Eric W. Biederman
> >> > <ebiederm@xmission.com> wrote:
> >> >> Guenter Roeck <linux@roeck-us.net> writes:
> >> >>
> >> >>> What I know so far is
> >> >>> - We see this condition on a regular basis in the field. Regular is
> >> >>>   relative, of course - let's say maybe 1 in a Milion Chromebooks
> >> >>>   per day reports a crash because of it. That is not that many,
> >> >>>   but it adds up.
> >> >>> - We are able to reproduce the problem with a performance benchmark
> >> >>>   which opens 100 chrome tabs. While that is a lot, it should not
> >> >>>   result in a kernel hang/crash.
> >> >>> - Vovo proviced the test code last night. I don't know if this is
> >> >>>   exactly what is observed in the benchmark, or how it relates to the
> >> >>>   benchmark in the first place, but it is the first time we are actually
> >> >>>   able to reliably create a condition where the problem is seen.
> >> >>
> >> >> Thank you.  I will be interesting to hear what is happening in the
> >> >> chrome perfomance benchmark that triggers this.
> >> >>
> >> > What's happening in the benchmark:
> >> > 1. A chrome renderer process was created with CLONE_NEWPID
> >> > 2. The process crashed
> >> > 3. Chrome breakpad service calls ptrace(PTRACE_ATTACH, ..) to attach to every
> >> >   threads of the crashed process to dump info
> >> > 4. When breakpad detach the crashed process, the crashed process stuck in
> >> >   zap_pid_ns_processes()
> >> 
> >> Very interesting thank you.
> >> 
> >> So the question is specifically which interaction is causing this.
> >> 
> >> In the test case provided it was a sibling task in the pid namespace
> >> dying and not being reaped.  Which may be what is happening with
> >> breakpad.  So far I have yet to see kernel bug but I won't rule one out.
> >> 
> >
> > I am trying to understand what you are looking for. I would have thought
> > that both the test application as well as the Chrome functionality
> > described above show that there are situations where zap_pid_ns_processes()
> > can get stuck and cause hung task timeouts in conjunction with the use of
> > ptrace().
> >
> > Your last sentence seems to suggest that you believe that the kernel might
> > do what it is expected to do. Assuming this is the case, what else would
> > you like to see ? A test application which matches exactly the Chrome use
> > case ? We can try to provide that, but I don't entirely understand how
> > that would change the situation. After all, we already know that it is
> > possible to get a thread into this condition, and we already have one
> > means to reproduce it.
> >
> > Replacing TASK_UNINTERRUPTIBLE with TASK_INTERRUPTABLE works for both the
> > test application and the Chrome benchmark. The thread is still stuck in
> > zap_pid_ns_processes(), but it is now in S (sleep) state instead of D,
> > and no longer results in a hung task timeout. It remains in that state
> > until the parent process terminates. I am not entirely happy with it
> > since the processes are still stuck and may pile up over time, but at
> > least it solves the immediate problem for us.
> >
> > Question now is what to do with that solution. We can of course apply
> > it locally to Chrome OS, but I would rather have it upstream - especially
> > since we have to assume that any users of Chrome on Linux, or more
> > generically anyone using ptrace in conjunction with CLONE_NEWPID, may
> > experience the same problem. Right now I have no idea how to get there,
> > though. Can you provide some guidance ?
> 
> Apologies for not being clear.  I intend to send a pull request with the

No worries.

> the TASK_UINTERRUPTIBLE to TASK_INTERRUPTIBLE change to Linus in the
> next week or so with a Cc stable and an appropriate Fixes tag.  So the
> fix can be backported.
> 
Great, that is good to know.

> I have a more comprehensive change queued I will probably merge for 4.13
> already but it just changes what kind of zombies you see.  It won't
> remove the ``stuck'' zombies.
> 
> So what I am looking for now is:
> Why are things getting stuck in your benchmark?
> 
> -  Is it a userspace bug?
> 
>   In which case we can figure out what userspace (aka breakpad) needs
>    to do to avoid the problem.
>    
I spent some time trying to understand what breakpad is doing. I don't
really see how it can be blamed. It attaches itself to a crashing thread,
gets the information it is looking for, and detaches itself. At the
same time, whatever it does, it seems to me that userspace should not
be able to create such a situation in the first place.

> -  Is it a kernel bug with ptrace?
> 
>    There have been a lot of little subtle bugs with ptrace over the
>    years so one more would not surprise
> 
Good question. I suspect that PTRACE_ATTACH (or PTRACE_TRACEME)
changes the reaper to be the calling process, and a subsequent
PTRACE_DETACH or exit of the process calling PTRACE_TRACEME doesn't
change it back or changes it to the process group owner. Is that
what happens, and if so is it what should happen ? I don't know (yet).

> So I am just looking to make certain we fix the root issue not just
> the hung task timeout warning.
> 
Makes sense. At least with TASK_INTERRUPTIBLE, I suspect we are just
fixing the symptom, but on the other side that isn't really my area
of expertise, so I don't really know.

One additional data point: If I time the threads in the test code by
adding various calls to sleep(), it is still more or less random if the
task status ends up in Z or D, meaning the child thread is sometimes
reaped and sometimes not. I would have expected it to be well defined.
I may be wrong, but that smells racy to me.

Anyway, I'll spend some more time on this. I'll let you know if I find
anything.

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1640725

Guenter Roeck <linux@roeck-us.net> writes:

> Hi Eric,
>
> On Fri, May 12, 2017 at 12:33:01PM -0500, Eric W. Biederman wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:
>> 
>> > Hi Eric,
>> >
>> > On Fri, May 12, 2017 at 08:26:27AM -0500, Eric W. Biederman wrote:
>> >> Vovo Yang <vovoy@google.com> writes:
>> >> 
>> >> > On Fri, May 12, 2017 at 7:19 AM, Eric W. Biederman
>> >> > <ebiederm@xmission.com> wrote:
>> >> >> Guenter Roeck <linux@roeck-us.net> writes:
>> >> >>
>> >> >>> What I know so far is
>> >> >>> - We see this condition on a regular basis in the field. Regular is
>> >> >>>   relative, of course - let's say maybe 1 in a Milion Chromebooks
>> >> >>>   per day reports a crash because of it. That is not that many,
>> >> >>>   but it adds up.
>> >> >>> - We are able to reproduce the problem with a performance benchmark
>> >> >>>   which opens 100 chrome tabs. While that is a lot, it should not
>> >> >>>   result in a kernel hang/crash.
>> >> >>> - Vovo proviced the test code last night. I don't know if this is
>> >> >>>   exactly what is observed in the benchmark, or how it relates to the
>> >> >>>   benchmark in the first place, but it is the first time we are actually
>> >> >>>   able to reliably create a condition where the problem is seen.
>> >> >>
>> >> >> Thank you.  I will be interesting to hear what is happening in the
>> >> >> chrome perfomance benchmark that triggers this.
>> >> >>
>> >> > What's happening in the benchmark:
>> >> > 1. A chrome renderer process was created with CLONE_NEWPID
>> >> > 2. The process crashed
>> >> > 3. Chrome breakpad service calls ptrace(PTRACE_ATTACH, ..) to attach to every
>> >> >   threads of the crashed process to dump info
>> >> > 4. When breakpad detach the crashed process, the crashed process stuck in
>> >> >   zap_pid_ns_processes()
>> >> 
>> >> Very interesting thank you.
>> >> 
>> >> So the question is specifically which interaction is causing this.
>> >> 
>> >> In the test case provided it was a sibling task in the pid namespace
>> >> dying and not being reaped.  Which may be what is happening with
>> >> breakpad.  So far I have yet to see kernel bug but I won't rule one out.
>> >> 
>> >
>> > I am trying to understand what you are looking for. I would have thought
>> > that both the test application as well as the Chrome functionality
>> > described above show that there are situations where zap_pid_ns_processes()
>> > can get stuck and cause hung task timeouts in conjunction with the use of
>> > ptrace().
>> >
>> > Your last sentence seems to suggest that you believe that the kernel might
>> > do what it is expected to do. Assuming this is the case, what else would
>> > you like to see ? A test application which matches exactly the Chrome use
>> > case ? We can try to provide that, but I don't entirely understand how
>> > that would change the situation. After all, we already know that it is
>> > possible to get a thread into this condition, and we already have one
>> > means to reproduce it.
>> >
>> > Replacing TASK_UNINTERRUPTIBLE with TASK_INTERRUPTABLE works for both the
>> > test application and the Chrome benchmark. The thread is still stuck in
>> > zap_pid_ns_processes(), but it is now in S (sleep) state instead of D,
>> > and no longer results in a hung task timeout. It remains in that state
>> > until the parent process terminates. I am not entirely happy with it
>> > since the processes are still stuck and may pile up over time, but at
>> > least it solves the immediate problem for us.
>> >
>> > Question now is what to do with that solution. We can of course apply
>> > it locally to Chrome OS, but I would rather have it upstream - especially
>> > since we have to assume that any users of Chrome on Linux, or more
>> > generically anyone using ptrace in conjunction with CLONE_NEWPID, may
>> > experience the same problem. Right now I have no idea how to get there,
>> > though. Can you provide some guidance ?
>> 
>> Apologies for not being clear.  I intend to send a pull request with the
>
> No worries.
>
>> the TASK_UINTERRUPTIBLE to TASK_INTERRUPTIBLE change to Linus in the
>> next week or so with a Cc stable and an appropriate Fixes tag.  So the
>> fix can be backported.
>> 
> Great, that is good to know.
>
>> I have a more comprehensive change queued I will probably merge for 4.13
>> already but it just changes what kind of zombies you see.  It won't
>> remove the ``stuck'' zombies.
>> 
>> So what I am looking for now is:
>> Why are things getting stuck in your benchmark?
>> 
>> -  Is it a userspace bug?
>> 
>>   In which case we can figure out what userspace (aka breakpad) needs
>>    to do to avoid the problem.
>>    
> I spent some time trying to understand what breakpad is doing. I don't
> really see how it can be blamed. It attaches itself to a crashing thread,
> gets the information it is looking for, and detaches itself. At the
> same time, whatever it does, it seems to me that userspace should not
> be able to create such a situation in the first place.

I suspect what is happening is that the thread breakpad is attached to
is killed by zap_pid_ns_processes.  At which point PTRACE_DETACH won't
work because the thread has been killed and waitpid(thread_pid,...)
needs to be used instead.

Certainly that is what is happening in the reproducer.

So I suspect breakpad just needs a waitpid whend PTRACE_DETACH fails
or possible just a general loop that listens for SIGCHLD and calls
some variant of wait.

>> -  Is it a kernel bug with ptrace?
>> 
>>    There have been a lot of little subtle bugs with ptrace over the
>>    years so one more would not surprise
>> 
> Good question. I suspect that PTRACE_ATTACH (or PTRACE_TRACEME)
> changes the reaper to be the calling process, and a subsequent
> PTRACE_DETACH or exit of the process calling PTRACE_TRACEME doesn't
> change it back or changes it to the process group owner. Is that
> what happens, and if so is it what should happen ? I don't know (yet).

Not the reaper but ptrace does change the parent to be the tracer.
PTRACE_DETACH won't work if the process is not stopped waiting
for the tracer.  And being killed and being a zombie is the ultimate
not waiting for the tracer state.

What needs to happen is for the tracer (aka breakpad) to call waitpid.

>> So I am just looking to make certain we fix the root issue not just
>> the hung task timeout warning.
>> 
> Makes sense. At least with TASK_INTERRUPTIBLE, I suspect we are just
> fixing the symptom, but on the other side that isn't really my area
> of expertise, so I don't really know.
>
> One additional data point: If I time the threads in the test code by
> adding various calls to sleep(), it is still more or less random if the
> task status ends up in Z or D, meaning the child thread is sometimes
> reaped and sometimes not. I would have expected it to be well defined.
> I may be wrong, but that smells racy to me.

If you have the threads exit with the exit system call rather than
what in the kernel winds up being do_group_exit it won't be racy.
The easy version is to use pthreads from child1 to create child2
and have them exit with pthread_exit.

Alternatively you can wrap call syscall(SYS_exit, exit_code); which
should also do the trick.

> Anyway, I'll spend some more time on this. I'll let you know if I find
> anything.

Thanks.

Eric

[toc] | [prev] | [next] | [standalone]

#1640932

On 05/12/2017 01:03 PM, Eric W. Biederman wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
>
>> Hi Eric,
>>
>> On Fri, May 12, 2017 at 12:33:01PM -0500, Eric W. Biederman wrote:
>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>
>>>> Hi Eric,
>>>>
>>>> On Fri, May 12, 2017 at 08:26:27AM -0500, Eric W. Biederman wrote:
>>>>> Vovo Yang <vovoy@google.com> writes:
>>>>>
>>>>>> On Fri, May 12, 2017 at 7:19 AM, Eric W. Biederman
>>>>>> <ebiederm@xmission.com> wrote:
>>>>>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>>>>>
>>>>>>>> What I know so far is
>>>>>>>> - We see this condition on a regular basis in the field. Regular is
>>>>>>>>   relative, of course - let's say maybe 1 in a Milion Chromebooks
>>>>>>>>   per day reports a crash because of it. That is not that many,
>>>>>>>>   but it adds up.
>>>>>>>> - We are able to reproduce the problem with a performance benchmark
>>>>>>>>   which opens 100 chrome tabs. While that is a lot, it should not
>>>>>>>>   result in a kernel hang/crash.
>>>>>>>> - Vovo proviced the test code last night. I don't know if this is
>>>>>>>>   exactly what is observed in the benchmark, or how it relates to the
>>>>>>>>   benchmark in the first place, but it is the first time we are actually
>>>>>>>>   able to reliably create a condition where the problem is seen.
>>>>>>>
>>>>>>> Thank you.  I will be interesting to hear what is happening in the
>>>>>>> chrome perfomance benchmark that triggers this.
>>>>>>>
>>>>>> What's happening in the benchmark:
>>>>>> 1. A chrome renderer process was created with CLONE_NEWPID
>>>>>> 2. The process crashed
>>>>>> 3. Chrome breakpad service calls ptrace(PTRACE_ATTACH, ..) to attach to every
>>>>>>   threads of the crashed process to dump info
>>>>>> 4. When breakpad detach the crashed process, the crashed process stuck in
>>>>>>   zap_pid_ns_processes()
>>>>>
>>>>> Very interesting thank you.
>>>>>
>>>>> So the question is specifically which interaction is causing this.
>>>>>
>>>>> In the test case provided it was a sibling task in the pid namespace
>>>>> dying and not being reaped.  Which may be what is happening with
>>>>> breakpad.  So far I have yet to see kernel bug but I won't rule one out.
>>>>>
>>>>
>>>> I am trying to understand what you are looking for. I would have thought
>>>> that both the test application as well as the Chrome functionality
>>>> described above show that there are situations where zap_pid_ns_processes()
>>>> can get stuck and cause hung task timeouts in conjunction with the use of
>>>> ptrace().
>>>>
>>>> Your last sentence seems to suggest that you believe that the kernel might
>>>> do what it is expected to do. Assuming this is the case, what else would
>>>> you like to see ? A test application which matches exactly the Chrome use
>>>> case ? We can try to provide that, but I don't entirely understand how
>>>> that would change the situation. After all, we already know that it is
>>>> possible to get a thread into this condition, and we already have one
>>>> means to reproduce it.
>>>>
>>>> Replacing TASK_UNINTERRUPTIBLE with TASK_INTERRUPTABLE works for both the
>>>> test application and the Chrome benchmark. The thread is still stuck in
>>>> zap_pid_ns_processes(), but it is now in S (sleep) state instead of D,
>>>> and no longer results in a hung task timeout. It remains in that state
>>>> until the parent process terminates. I am not entirely happy with it
>>>> since the processes are still stuck and may pile up over time, but at
>>>> least it solves the immediate problem for us.
>>>>
>>>> Question now is what to do with that solution. We can of course apply
>>>> it locally to Chrome OS, but I would rather have it upstream - especially
>>>> since we have to assume that any users of Chrome on Linux, or more
>>>> generically anyone using ptrace in conjunction with CLONE_NEWPID, may
>>>> experience the same problem. Right now I have no idea how to get there,
>>>> though. Can you provide some guidance ?
>>>
>>> Apologies for not being clear.  I intend to send a pull request with the
>>
>> No worries.
>>
>>> the TASK_UINTERRUPTIBLE to TASK_INTERRUPTIBLE change to Linus in the
>>> next week or so with a Cc stable and an appropriate Fixes tag.  So the
>>> fix can be backported.
>>>
>> Great, that is good to know.
>>
>>> I have a more comprehensive change queued I will probably merge for 4.13
>>> already but it just changes what kind of zombies you see.  It won't
>>> remove the ``stuck'' zombies.
>>>
>>> So what I am looking for now is:
>>> Why are things getting stuck in your benchmark?
>>>
>>> -  Is it a userspace bug?
>>>
>>>   In which case we can figure out what userspace (aka breakpad) needs
>>>    to do to avoid the problem.
>>>
>> I spent some time trying to understand what breakpad is doing. I don't
>> really see how it can be blamed. It attaches itself to a crashing thread,
>> gets the information it is looking for, and detaches itself. At the
>> same time, whatever it does, it seems to me that userspace should not
>> be able to create such a situation in the first place.
>
> I suspect what is happening is that the thread breakpad is attached to
> is killed by zap_pid_ns_processes.  At which point PTRACE_DETACH won't
> work because the thread has been killed and waitpid(thread_pid,...)
> needs to be used instead.
>

I think you nailed it. If I drop CLONE_NEWPID from the reproducer I get
a zombie process.

I guess the only question left is if zap_pid_ns_processes() should (or could)
somehow detect that situation and return instead of waiting forever.
What do you think ?

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1640975

Guenter Roeck <linux@roeck-us.net> writes:

> On 05/12/2017 01:03 PM, Eric W. Biederman wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:
>>
>>> Hi Eric,
>>>
>>> On Fri, May 12, 2017 at 12:33:01PM -0500, Eric W. Biederman wrote:
>>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>>
>>>>> Hi Eric,
>>>>>
>>>>> On Fri, May 12, 2017 at 08:26:27AM -0500, Eric W. Biederman wrote:
>>>>>> Vovo Yang <vovoy@google.com> writes:
>>>>>>
>>>>>>> On Fri, May 12, 2017 at 7:19 AM, Eric W. Biederman
>>>>>>> <ebiederm@xmission.com> wrote:
>>>>>>>> Guenter Roeck <linux@roeck-us.net> writes:
>>>>>>>>
>>>>>>>>> What I know so far is
>>>>>>>>> - We see this condition on a regular basis in the field. Regular is
>>>>>>>>>   relative, of course - let's say maybe 1 in a Milion Chromebooks
>>>>>>>>>   per day reports a crash because of it. That is not that many,
>>>>>>>>>   but it adds up.
>>>>>>>>> - We are able to reproduce the problem with a performance benchmark
>>>>>>>>>   which opens 100 chrome tabs. While that is a lot, it should not
>>>>>>>>>   result in a kernel hang/crash.
>>>>>>>>> - Vovo proviced the test code last night. I don't know if this is
>>>>>>>>>   exactly what is observed in the benchmark, or how it relates to the
>>>>>>>>>   benchmark in the first place, but it is the first time we are actually
>>>>>>>>>   able to reliably create a condition where the problem is seen.
>>>>>>>>
>>>>>>>> Thank you.  I will be interesting to hear what is happening in the
>>>>>>>> chrome perfomance benchmark that triggers this.
>>>>>>>>
>>>>>>> What's happening in the benchmark:
>>>>>>> 1. A chrome renderer process was created with CLONE_NEWPID
>>>>>>> 2. The process crashed
>>>>>>> 3. Chrome breakpad service calls ptrace(PTRACE_ATTACH, ..) to attach to every
>>>>>>>   threads of the crashed process to dump info
>>>>>>> 4. When breakpad detach the crashed process, the crashed process stuck in
>>>>>>>   zap_pid_ns_processes()
>>>>>>
>>>>>> Very interesting thank you.
>>>>>>
>>>>>> So the question is specifically which interaction is causing this.
>>>>>>
>>>>>> In the test case provided it was a sibling task in the pid namespace
>>>>>> dying and not being reaped.  Which may be what is happening with
>>>>>> breakpad.  So far I have yet to see kernel bug but I won't rule one out.
>>>>>>
>>>>>
>>>>> I am trying to understand what you are looking for. I would have thought
>>>>> that both the test application as well as the Chrome functionality
>>>>> described above show that there are situations where zap_pid_ns_processes()
>>>>> can get stuck and cause hung task timeouts in conjunction with the use of
>>>>> ptrace().
>>>>>
>>>>> Your last sentence seems to suggest that you believe that the kernel might
>>>>> do what it is expected to do. Assuming this is the case, what else would
>>>>> you like to see ? A test application which matches exactly the Chrome use
>>>>> case ? We can try to provide that, but I don't entirely understand how
>>>>> that would change the situation. After all, we already know that it is
>>>>> possible to get a thread into this condition, and we already have one
>>>>> means to reproduce it.
>>>>>
>>>>> Replacing TASK_UNINTERRUPTIBLE with TASK_INTERRUPTABLE works for both the
>>>>> test application and the Chrome benchmark. The thread is still stuck in
>>>>> zap_pid_ns_processes(), but it is now in S (sleep) state instead of D,
>>>>> and no longer results in a hung task timeout. It remains in that state
>>>>> until the parent process terminates. I am not entirely happy with it
>>>>> since the processes are still stuck and may pile up over time, but at
>>>>> least it solves the immediate problem for us.
>>>>>
>>>>> Question now is what to do with that solution. We can of course apply
>>>>> it locally to Chrome OS, but I would rather have it upstream - especially
>>>>> since we have to assume that any users of Chrome on Linux, or more
>>>>> generically anyone using ptrace in conjunction with CLONE_NEWPID, may
>>>>> experience the same problem. Right now I have no idea how to get there,
>>>>> though. Can you provide some guidance ?
>>>>
>>>> Apologies for not being clear.  I intend to send a pull request with the
>>>
>>> No worries.
>>>
>>>> the TASK_UINTERRUPTIBLE to TASK_INTERRUPTIBLE change to Linus in the
>>>> next week or so with a Cc stable and an appropriate Fixes tag.  So the
>>>> fix can be backported.
>>>>
>>> Great, that is good to know.
>>>
>>>> I have a more comprehensive change queued I will probably merge for 4.13
>>>> already but it just changes what kind of zombies you see.  It won't
>>>> remove the ``stuck'' zombies.
>>>>
>>>> So what I am looking for now is:
>>>> Why are things getting stuck in your benchmark?
>>>>
>>>> -  Is it a userspace bug?
>>>>
>>>>   In which case we can figure out what userspace (aka breakpad) needs
>>>>    to do to avoid the problem.
>>>>
>>> I spent some time trying to understand what breakpad is doing. I don't
>>> really see how it can be blamed. It attaches itself to a crashing thread,
>>> gets the information it is looking for, and detaches itself. At the
>>> same time, whatever it does, it seems to me that userspace should not
>>> be able to create such a situation in the first place.
>>
>> I suspect what is happening is that the thread breakpad is attached to
>> is killed by zap_pid_ns_processes.  At which point PTRACE_DETACH won't
>> work because the thread has been killed and waitpid(thread_pid,...)
>> needs to be used instead.
>>
>
> I think you nailed it. If I drop CLONE_NEWPID from the reproducer I get
> a zombie process.
>
> I guess the only question left is if zap_pid_ns_processes() should (or could)
> somehow detect that situation and return instead of waiting forever.
> What do you think ?

In this case zap_pid_ns_processes is waiting for everything to be done
with the processes in the pid namespace.  As something outside the
namespace is tracing processes inside the namespace things they aren't
done with the processes in the pid namespace.

So in general I think the current behavior is correct.  If the tracer
(breakpad?) had set SA_NOCLDWAIT or set SIGCHLD to SIG_IGN I would
figure the failure as a kernel bug.

At this point it just appears that the tracer has a bug (ignoring
SIGCHLD and not reaping dead children) that should be fixed.

Eric

[toc] | [prev] | [next] | [standalone]

#1640176

Guenter Roeck <linux@roeck-us.net> writes:

> On Thu, May 11, 2017 at 04:25:23PM -0500, Eric W. Biederman wrote:
>> Guenter Roeck <linux@roeck-us.net> writes:

>> > 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.
>> 
>
> Almost. Something along the line of
>
> 	count = 0;
> 	nr = next_pidmap(pid_ns, 0);
> 	while (nr > 0) {
> 		count++;
> 		nr = next_pidmap(pid_ns, nr);
> 	}
>
> only I also call sched_show_task() for each thread, and the only
> one printed is the one that hangs in zap_pid_ns_processes().

The function sched_show_task() does:
	if (!try_get_task_stack(p))
        	return;
                
Which won't work on a zombie who has already released it's stack.
Which is exactly what child2 should be at that point.

Eric

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web