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

Groups > linux.kernel > #1440284 > unrolled thread

Re: bug in memcg oom-killer results in a hung syscall in another process in the same cgroup

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: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Michal Hocko <mhocko@kernel.org> - 2016-07-11 08:50 +0200
    Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Shayan Pooya <shayan@liveve.org> - 2016-07-11 20:40 +0200
      Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Michal Hocko <mhocko@kernel.org> - 2016-07-12 09:20 +0200
        Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Shayan Pooya <shayan@liveve.org> - 2016-07-12 17:40 +0200
          Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Konstantin Khlebnikov <khlebnikov@yandex-team.ru> - 2016-07-12 18:00 +0200
            Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Oleg Nesterov <oleg@redhat.com> - 2016-07-12 19:00 +0200
            Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Shayan Pooya <shayan@liveve.org> - 2016-07-13 01:10 +0200
              Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Oleg Nesterov <oleg@redhat.com> - 2016-07-14 15:30 +0200
                Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Shayan Pooya <shayan@liveve.org> - 2016-07-14 17:40 +0200
                  Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Shayan Pooya <shayan@liveve.org> - 2016-07-15 19:00 +0200
                    Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Oleg Nesterov <oleg@redhat.com> - 2016-07-18 16:00 +0200
          Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Michal Hocko <mhocko@kernel.org> - 2016-07-13 10:10 +0200
    Re: bug in memcg oom-killer results in a hung syscall in another  process in the same cgroup Michal Hocko <mhocko@kernel.org> - 2016-07-12 09:20 +0200

#1440284 — Re: bug in memcg oom-killer results in a hung syscall in another process in the same cgroup

On Sat 09-07-16 16:49:32, Shayan Pooya wrote:
> I came across the following issue in kernel 3.16 (Ubuntu 14.04) which
> was then reproduced in kernels 4.4 LTS:
> After a couple of of memcg oom-kills in a cgroup, a syscall in
> *another* process in the same cgroup hangs indefinitely.
> 
> Reproducing:
> 
> # mkdir -p strace_run
> #  mkdir /sys/fs/cgroup/memory/1
> # echo 1073741824 > /sys/fs/cgroup/memory/1/memory.limit_in_bytes
> # echo 0 > /sys/fs/cgroup/memory/1/memory.swappiness
> # for i in $(seq 1000); do ./call-mem-hog
> /sys/fs/cgroup/memory/1/cgroup.procs & done
> 
> Where call-mem-hog is:
> #!/bin/sh
> set -ex
> echo $$ > $1
> echo "Adding $$ to $1"
> strace -ff -tt ./mem-hog 2> strace_run/$$
> 
> 
> Initially I thought it was a userspace bug in dash as it only happened
> with /bin/sh (which points to dash) and not with bash. I see the
> following hanging processes:
> 
> USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
> root     20999  0.0  0.0   4508   100 pts/6    S    16:28   0:00
> /bin/sh ./call-mem-hog /sys/fs/cgroup/memory/1/cgroup.procs
> 
> However, when using strace, I noticed that sometimes there is actually
> a mem-hog process hanging on sbrk syscall (Of course the
> memory.oom_control is 0 and this is not expected).
> Sending an ABRT signal to the waiting strace process then resulted in
> the mem-hog process getting oom-killed by the kernel.

Could you post the stack trace of the hung oom victim? Also could you
post the full kernel log?
-- 
Michal Hocko
SUSE Labs

[toc] | [next] | [standalone]

#1440812

>> Could you post the stack trace of the hung oom victim? Also could you
>> post the full kernel log?

With strace, when running 500 concurrent mem-hog tasks on the same
kernel, 33 of them failed with:

strace: ../sysdeps/nptl/fork.c:136: __libc_fork: Assertion
`THREAD_GETMEM (self, tid) != ppid' failed.

Which is: https://sourceware.org/bugzilla/show_bug.cgi?id=15392
And discussed before at: https://lkml.org/lkml/2015/2/6/470 but that
patch was not accepted.

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

#1441106

On Mon 11-07-16 11:33:19, Shayan Pooya wrote:
> >> Could you post the stack trace of the hung oom victim? Also could you
> >> post the full kernel log?
> 
> With strace, when running 500 concurrent mem-hog tasks on the same
> kernel, 33 of them failed with:
> 
> strace: ../sysdeps/nptl/fork.c:136: __libc_fork: Assertion
> `THREAD_GETMEM (self, tid) != ppid' failed.
> 
> Which is: https://sourceware.org/bugzilla/show_bug.cgi?id=15392
> And discussed before at: https://lkml.org/lkml/2015/2/6/470 but that
> patch was not accepted.

OK, so the problem is that the oom killed task doesn't report the futex
release properly? If yes then I fail to see how that is memcg specific.
Could you try to clarify what you consider a bug again, please? I am not
really sure I understand this report.

Thanks!

-- 
Michal Hocko
SUSE Labs

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

#1441516

>> With strace, when running 500 concurrent mem-hog tasks on the same
>> kernel, 33 of them failed with:
>>
>> strace: ../sysdeps/nptl/fork.c:136: __libc_fork: Assertion
>> `THREAD_GETMEM (self, tid) != ppid' failed.
>>
>> Which is: https://sourceware.org/bugzilla/show_bug.cgi?id=15392
>> And discussed before at: https://lkml.org/lkml/2015/2/6/470 but that
>> patch was not accepted.
>
> OK, so the problem is that the oom killed task doesn't report the futex
> release properly? If yes then I fail to see how that is memcg specific.
> Could you try to clarify what you consider a bug again, please? I am not
> really sure I understand this report.

It looks like it is just a very easy way to reproduce the problem that
Konstantin described in that lkml thread. That patch was not accepted
and I see no other fixes for that issue upstream. Here is a copy of
his root-cause analysis from said thread:

Whole sequence looks like: task calls fork, glibc calls syscall clone with
CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF->tid as argument.
Child task gets read-only copy of VM including TLS. Child calls put_user()
to handle CLONE_CHILD_SETTID from schedule_tail(). put_user() trigger page
fault and it fails because do_wp_page()  hits memcg limit without invoking
OOM-killer because this is page-fault from kernel-space.  Put_user returns
-EFAULT, which is ignored.  Child returns into user-space and catches here
assert (THREAD_GETMEM (self, tid) != ppid), glibc tries to print something
but hangs on deadlock on internal locks. Halt and catch fire.



Regards

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

#1441522

On 12.07.2016 18:35, Shayan Pooya wrote:
>>> With strace, when running 500 concurrent mem-hog tasks on the same
>>> kernel, 33 of them failed with:
>>>
>>> strace: ../sysdeps/nptl/fork.c:136: __libc_fork: Assertion
>>> `THREAD_GETMEM (self, tid) != ppid' failed.
>>>
>>> Which is: https://sourceware.org/bugzilla/show_bug.cgi?id=15392
>>> And discussed before at: https://lkml.org/lkml/2015/2/6/470 but that
>>> patch was not accepted.
>>
>> OK, so the problem is that the oom killed task doesn't report the futex
>> release properly? If yes then I fail to see how that is memcg specific.
>> Could you try to clarify what you consider a bug again, please? I am not
>> really sure I understand this report.
>
> It looks like it is just a very easy way to reproduce the problem that
> Konstantin described in that lkml thread. That patch was not accepted
> and I see no other fixes for that issue upstream. Here is a copy of
> his root-cause analysis from said thread:
>
> Whole sequence looks like: task calls fork, glibc calls syscall clone with
> CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF->tid as argument.
> Child task gets read-only copy of VM including TLS. Child calls put_user()
> to handle CLONE_CHILD_SETTID from schedule_tail(). put_user() trigger page
> fault and it fails because do_wp_page()  hits memcg limit without invoking
> OOM-killer because this is page-fault from kernel-space.  Put_user returns
> -EFAULT, which is ignored.  Child returns into user-space and catches here
> assert (THREAD_GETMEM (self, tid) != ppid), glibc tries to print something
> but hangs on deadlock on internal locks. Halt and catch fire.
>
>

Yep. Bug still not fixed in upstream. In our kernel I've plugged it with this:

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2808,8 +2808,9 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)
         balance_callback(rq);
         preempt_enable();

-       if (current->set_child_tid)
-               put_user(task_pid_vnr(current), current->set_child_tid);
+       if (current->set_child_tid &&
+           put_user(task_pid_vnr(current), current->set_child_tid))
+               force_sig(SIGSEGV, current);
  }

Add Oleg into CC. IIRR he had some ideas how to fix this. =)

-- 
Konstantin

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

#1441592

On 07/12, Konstantin Khlebnikov wrote:
>
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2808,8 +2808,9 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)
>         balance_callback(rq);
>         preempt_enable();
>
> -       if (current->set_child_tid)
> -               put_user(task_pid_vnr(current), current->set_child_tid);
> +       if (current->set_child_tid &&
> +           put_user(task_pid_vnr(current), current->set_child_tid))
> +               force_sig(SIGSEGV, current);
>  }
>
> Add Oleg into CC. IIRR he had some ideas how to fix this. =)

Heh. OK, OK, thank you Konstantin ;)

I'll try to recall tomorrow, but iirc I only have some ideas of how
we can happily blame the FAULT_FLAG_USER logic.

d, in this particular case, perhaps glibc/set_child_tid too because
(again, iirc) it would nice to simply kill it, it is only used for
some sanity checks...

Oleg.

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

#1441900

> Yep. Bug still not fixed in upstream. In our kernel I've plugged it with
> this:
>
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2808,8 +2808,9 @@ asmlinkage __visible void schedule_tail(struct
> task_struct *prev)
>         balance_callback(rq);
>         preempt_enable();
>
> -       if (current->set_child_tid)
> -               put_user(task_pid_vnr(current), current->set_child_tid);
> +       if (current->set_child_tid &&
> +           put_user(task_pid_vnr(current), current->set_child_tid))
> +               force_sig(SIGSEGV, current);
>  }

I just verified that with your patch there is no hung processes and I
see processes getting SIGSEGV as expected.
Thanks!

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

#1443451

On 07/12, Shayan Pooya wrote:
>
> > Yep. Bug still not fixed in upstream. In our kernel I've plugged it with
> > this:
> >
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -2808,8 +2808,9 @@ asmlinkage __visible void schedule_tail(struct
> > task_struct *prev)
> >         balance_callback(rq);
> >         preempt_enable();
> >
> > -       if (current->set_child_tid)
> > -               put_user(task_pid_vnr(current), current->set_child_tid);
> > +       if (current->set_child_tid &&
> > +           put_user(task_pid_vnr(current), current->set_child_tid))
> > +               force_sig(SIGSEGV, current);
> >  }
>
> I just verified that with your patch there is no hung processes and I
> see processes getting SIGSEGV as expected.

Well, but we can't do this. And "as expected" is actually just wrong. I still
think that the whole FAULT_FLAG_USER logic is not right. This needs another email.

fork() should not fail because there is a memory hog in the same memcg. Worse,
pthread_create() can kill the caller by the same reason. And we have the same
or even worse problem with ->clear_child_tid, pthread_join() can hang forever.
Unlikely we want to kill the application in this case ;)

And in fact I think that the problem has nothing to do with set/claer_child_tid
in particular.

I am just curious... can you reproduce the problem reliably? If yes, can you try
the patch below ? Just in case, this is not the real fix in any case...

Oleg.

--- x/kernel/sched/core.c
+++ x/kernel/sched/core.c
@@ -2793,8 +2793,11 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)
 	balance_callback(rq);
 	preempt_enable();
 
-	if (current->set_child_tid)
+	if (current->set_child_tid) {
+		mem_cgroup_oom_enable();
 		put_user(task_pid_vnr(current), current->set_child_tid);
+		mem_cgroup_oom_disable();
+	}
 }
 
 /*

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

#1443542

> Well, but we can't do this. And "as expected" is actually just wrong. I still
> think that the whole FAULT_FLAG_USER logic is not right. This needs another email.

I meant as expected from the content of the patch :) I think
Konstantin agrees that this patch cannot be merged upstream.

> fork() should not fail because there is a memory hog in the same memcg. Worse,
> pthread_create() can kill the caller by the same reason. And we have the same
> or even worse problem with ->clear_child_tid, pthread_join() can hang forever.
> Unlikely we want to kill the application in this case ;)
>
> And in fact I think that the problem has nothing to do with set/claer_child_tid
> in particular.
>
> I am just curious... can you reproduce the problem reliably? If yes, can you try
> the patch below ? Just in case, this is not the real fix in any case...

Yes. It deterministically results in hung processes in vanilla kernel.
I'll try this patch.


> --- x/kernel/sched/core.c
> +++ x/kernel/sched/core.c
> @@ -2793,8 +2793,11 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)
>         balance_callback(rq);
>         preempt_enable();
>
> -       if (current->set_child_tid)
> +       if (current->set_child_tid) {
> +               mem_cgroup_oom_enable();
>                 put_user(task_pid_vnr(current), current->set_child_tid);
> +               mem_cgroup_oom_disable();
> +       }
>  }
>
>  /*
>

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

#1444454

>> I am just curious... can you reproduce the problem reliably? If yes, can you try
>> the patch below ? Just in case, this is not the real fix in any case...
>
> Yes. It deterministically results in hung processes in vanilla kernel.
> I'll try this patch.

I'll have to correct this. I can reproduce this issue easily on
high-end servers and normal laptops. But for some reason it does not
happen very often in vmware guests (maybe related to lower
parallelism).

>> --- x/kernel/sched/core.c
>> +++ x/kernel/sched/core.c
>> @@ -2793,8 +2793,11 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)
>>         balance_callback(rq);
>>         preempt_enable();
>>
>> -       if (current->set_child_tid)
>> +       if (current->set_child_tid) {
>> +               mem_cgroup_oom_enable();
>>                 put_user(task_pid_vnr(current), current->set_child_tid);
>> +               mem_cgroup_oom_disable();
>> +       }
>>  }
>>
>>  /*

I tried this patch and I still see the same stuck processes (assuming
that's what you were curious about).

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

#1445499

On 07/15, Shayan Pooya wrote:
>
> >> --- x/kernel/sched/core.c
> >> +++ x/kernel/sched/core.c
> >> @@ -2793,8 +2793,11 @@ asmlinkage __visible void schedule_tail(struct task_struct *prev)
> >>         balance_callback(rq);
> >>         preempt_enable();
> >>
> >> -       if (current->set_child_tid)
> >> +       if (current->set_child_tid) {
> >> +               mem_cgroup_oom_enable();
> >>                 put_user(task_pid_vnr(current), current->set_child_tid);
> >> +               mem_cgroup_oom_disable();
> >> +       }
> >>  }
> >>
> >>  /*
>
> I tried this patch and I still see the same stuck processes (assuming
> that's what you were curious about).

Of course. Because I am stupid. Firtsly, I forgot to include another
change in fault.c. And now I see that change was wrong anyway.

I'll try to make another debugging patch today later, but let me repeat
that it won't fix the real problem anyway.

Thanks, and sorry for wasting your time.

Oleg.

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

#1442151

On Tue 12-07-16 08:35:06, Shayan Pooya wrote:
> >> With strace, when running 500 concurrent mem-hog tasks on the same
> >> kernel, 33 of them failed with:
> >>
> >> strace: ../sysdeps/nptl/fork.c:136: __libc_fork: Assertion
> >> `THREAD_GETMEM (self, tid) != ppid' failed.
> >>
> >> Which is: https://sourceware.org/bugzilla/show_bug.cgi?id=15392
> >> And discussed before at: https://lkml.org/lkml/2015/2/6/470 but that
> >> patch was not accepted.
> >
> > OK, so the problem is that the oom killed task doesn't report the futex
> > release properly? If yes then I fail to see how that is memcg specific.
> > Could you try to clarify what you consider a bug again, please? I am not
> > really sure I understand this report.
> 
> It looks like it is just a very easy way to reproduce the problem that
> Konstantin described in that lkml thread. That patch was not accepted
> and I see no other fixes for that issue upstream. Here is a copy of
> his root-cause analysis from said thread:
> 
> Whole sequence looks like: task calls fork, glibc calls syscall clone with
> CLONE_CHILD_SETTID and passes pointer to TLS THREAD_SELF->tid as argument.
> Child task gets read-only copy of VM including TLS. Child calls put_user()
> to handle CLONE_CHILD_SETTID from schedule_tail(). put_user() trigger page
> fault and it fails because do_wp_page()  hits memcg limit without invoking
> OOM-killer because this is page-fault from kernel-space.  Put_user returns
> -EFAULT, which is ignored.  Child returns into user-space and catches here
> assert (THREAD_GETMEM (self, tid) != ppid), glibc tries to print something
> but hangs on deadlock on internal locks. Halt and catch fire.

OK, I see! Thanks for the clarification. So the bug is that put_user
return value is ignored. Let's see whether Konstantin's patch will be
accepted or Oleg comes with something else.
-- 
Michal Hocko
SUSE Labs

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

#1441112

On Mon 11-07-16 10:40:55, Shayan Pooya wrote:
> >
> > Could you post the stack trace of the hung oom victim? Also could you
> > post the full kernel log?
> 
> Here is the stack of the process that lives (it is *not* the
> oom-victim) in a run with 100 processes and *without* strace:
> 
> # cat /proc/7688/stack
> [<ffffffff81100292>] futex_wait_queue_me+0xc2/0x120
> [<ffffffff811005a6>] futex_wait+0x116/0x280
> [<ffffffff81102d90>] do_futex+0x120/0x540
> [<ffffffff81103231>] SyS_futex+0x81/0x180
> [<ffffffff81825bf2>] entry_SYSCALL_64_fastpath+0x16/0x71
> [<ffffffffffffffff>] 0xffffffffffffffff

I am not sure I understand. Is this the hung task?
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web