linux-kernel - Re: memcg creates an unkillable task in 3.11-rc2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <874nbc3sx1.fsf@tw-ebiederman.twitter.com>
Date:	Tue, 30 Jul 2013 09:37:46 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Michal Hocko <mhocko@...e.cz>
Cc:	Li Zefan <lizefan@...wei.com>, Tejun Heo <tj@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	cgroups@...r.kernel.org, containers@...ts.linux-foundation.org,
	linux-kernel@...r.kernel.org, kent.overstreet@...il.com,
	Glauber Costa <glommer@...il.com>,
	Johannes Weiner <hannes@...xchg.org>
Subject: Re: memcg creates an unkillable task in 3.11-rc2

Michal Hocko <mhocko@...e.cz> writes:

> On Tue 30-07-13 01:19:31, Eric W. Biederman wrote:
> [...]
>> Hmm. Looking farther I see what is going on. And it has nothing to do
>> with the freezer. (I have commented out that code and reproduced it
>> without the freezer to be doubly certain).
>> 
>> 
>> On the exit path exit_robust_list is triggering a page fault to fault a
>> page back in.  Which since we have no memory causes the exit path
>> to get stuck in mem_cgroup_handle_oom.
>
> Hmm, interesting. I assume the exit is caused by the SIGKILL, right?
> If yes, then why it hasn't coughed early in __mem_cgroup_try_charge

Interesting question.  This isn't the primary thread but we do send
SIGKILL to the secondary threads as well.

We definitely need those checks on both paths making my change valid.

Oh. Duh!  This is after we act on SIGKILL so SIGKILL is no longer
pending.

That is how it makes it through __mem_cgroup_try_charge.


>> Which means the following change should fix the hang.  I will test it in just
>> a second.
>> 
>> The problem is that we only handled pending fatal signals and exiting
>> processes when the OOM logic was enabled. Sigh.
>
> If this is about PF_EXITING then we should handle that earlier in the
> code chain (__mem_cgroup_try_charge sounds like a proper place). Let me
> think about that.

We also need to test for fatal signals before we sleep in both boths of
mem_cgroup_out_of_memory.  But yes testing for this condition earlier
so we don't have to do a lot of work before we get out would not be a
bad thing.

Eric


>
> Thanks for debugging this!
>
>> Eric
>> 
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 00a7a66..5998a57 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -1792,16 +1792,6 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask,
>>         unsigned int points = 0;
>>         struct task_struct *chosen = NULL;
>>  
>> -       /*
>> -        * If current has a pending SIGKILL or is exiting, then automatically
>> -        * select it.  The goal is to allow it to allocate so that it may
>> -        * quickly exit and free its memory.
>> -        */
>> -       if (fatal_signal_pending(current) || current->flags & PF_EXITING) {
>> -               set_thread_flag(TIF_MEMDIE);
>> -               return;
>> -       }
>> -
>>         check_panic_on_oom(CONSTRAINT_MEMCG, gfp_mask, order, NULL);
>>         totalpages = mem_cgroup_get_limit(memcg) >> PAGE_SHIFT ? : 1;
>>         for_each_mem_cgroup_tree(iter, memcg) {
>> @@ -2220,7 +2210,15 @@ static bool mem_cgroup_handle_oom(struct mem_cgroup *memcg, gfp_t mask,
>>                 mem_cgroup_oom_notify(memcg);
>>         spin_unlock(&memcg_oom_lock);
>>  
>> -       if (need_to_kill) {
>> +       /*
>> +        * If current has a pending SIGKILL or is exiting, then automatically
>> +        * select it.  The goal is to allow it to allocate so that it may
>> +        * quickly exit and free its memory.
>> +        */
>> +       if (fatal_signal_pending(current) || current->flags & PF_EXITING) {
>> +               set_thread_flag(TIF_MEMDIE);
>> +               finish_wait(&memcg_oom_waitq, &owait.wait);
>> +       } else if (need_to_kill) {
>>                 finish_wait(&memcg_oom_waitq, &owait.wait);
>>                 mem_cgroup_out_of_memory(memcg, mask, order);
>>         } else {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/