linux-kernel - Re: + mm-oom-killer-kills-more-than-needed.patch added to -mm tree

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: <2137b82d0809231037w3900ef3fta003b791bf36d4a@mail.gmail.com>
Date:	Tue, 23 Sep 2008 10:37:52 -0700
From:	"Chad Zanonie" <chad.zanonie@...il.com>
To:	"Oleg Nesterov" <oleg@...sign.ru>
Cc:	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	npiggin@...e.de, rientjes@...gle.com
Subject: Re: + mm-oom-killer-kills-more-than-needed.patch added to -mm tree

Before I propagate this blunder anymore, I've found the root of my mistake.

I really mean TASK_DEAD, not EXIT_DEAD.

(p->state & TASK_DEAD)

-Chad

On Tue, Sep 23, 2008 at 10:25 AM, Chad Zanonie <chad.zanonie@...il.com> wrote:
> On Tue, Sep 23, 2008 at 5:40 AM, Oleg Nesterov <oleg@...sign.ru> wrote:
>> On 09/22, Andrew Morton wrote:
>>>
>>> ------------------------------------------------------
>>> Subject: mm: oom-killer kills more than needed
>>> From: "Chad Zanonie" <chad.zanonie@...il.com>
>>>
>>> Possibility exists for an exiting application to be in between marking its
>>> mm NULL and calling mmput when out_of_memory is invoked.
>>> select_bad_process() will continue past this process as opposed to
>>> returning -1UL due to its mm being NULL.  This causes the oom killer in
>>> certain scenarios to not only kill the memory culprit, but also kill the
>>> runner up.
>>>
>>> EXIT_DEAD seems to be the only flag that guarantees that mmput() has
>>> finished.
>>
>> I don't think this is right.
>>
>> Let's suppose we have a single zombie. Now select_bad_process() always
>> returns -1 ? IOW, doesn't this means that, say,
>>
>>        $ perl -e 'fork && sleep'
>>
>> disables oom-kill completely and forever?
>
> Ugh, looks like my description was slightly incorrect. I don't mean
> for it to return -1 upon noticing a null mm. I mean for
> select_bad_process to not skip (continue) over processes that haven't
> provably finished unmapping their memory (which EXIT_DEAD infers).
>
>>
>> Hmm. But please see below. This doesn't happen because the usage
>> of EXIT_DEAD is not right.
>>
>>> Checking for PF_KTHREAD should replace p->mm regardless.
>>
>> Yes, almost every check for ->mm in oom_kill.c is not right.
>>
>>> Adding EXIT_DEAD to the check seems to prevent unnecessary kills in local
>>> testing.
>>
>> This is strange, could you re-test? Because
>>
>>> @@ -216,7 +216,7 @@ static struct task_struct *select_bad_pr
>>>                * skip kernel threads and tasks which have already released
>>>                * their mm.
>>>                */
>>> -             if (!p->mm)
>>> +             if (p->flags & PF_KTHREAD || p->flags & EXIT_DEAD)
>>                                                ^^^^^^^^^^^^^^^^^
>>
>> this is not possible. EXIT_DEAD lives in ->exit_state, not in ->flags.
>
> Good catch. I had actually done the testing in an older kernel that
> used PF_DEAD and while noticing the change to only EXIT_DEAD forgot to
> use the new appropriate flag in the patch.
>
>>
>> Oleg.
>>
>>
>
> I'm new to this endeavor. Should I propose a new patch, or, will
> things be fixed from this omission? I still support the EXIT_DEAD
> inclusion, as it'll allow the OOM killer to make the similar progress
> that !p->mm provided.
>
> Chad
>
--
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/