Messages in this thread | | | Date | Tue, 23 Sep 2008 10:25:52 -0700 | From | "Chad Zanonie" <> | Subject | Re: + mm-oom-killer-kills-more-than-needed.patch added to -mm tree |
| |
On Tue, Sep 23, 2008 at 5:40 AM, Oleg Nesterov <oleg@tv-sign.ru> wrote: > On 09/22, Andrew Morton wrote: >> >> ------------------------------------------------------ >> Subject: mm: oom-killer kills more than needed >> From: "Chad Zanonie" <chad.zanonie@gmail.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
| |