All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Shakeel Butt <shakeelb@google.com>, linux-mm@kvack.org
Subject: Re: [PATCH] mm: mempolicy: don't select exited threads as OOM victims
Date: Mon, 1 Jul 2019 16:04:34 +0200	[thread overview]
Message-ID: <20190701140434.GA6376@dhcp22.suse.cz> (raw)
In-Reply-To: <a78dbba0-262e-87c5-e278-9e17cf9a63f7@i-love.sakura.ne.jp>

On Mon 01-07-19 22:56:12, Tetsuo Handa wrote:
> On 2019/07/01 22:48, Michal Hocko wrote:
> > On Mon 01-07-19 22:38:58, Tetsuo Handa wrote:
> >> On 2019/07/01 22:17, Michal Hocko wrote:
> >>> On Mon 01-07-19 22:04:22, Tetsuo Handa wrote:
> >>>> But I realized that this patch was too optimistic. We need to wait for mm-less
> >>>> threads until MMF_OOM_SKIP is set if the process was already an OOM victim.
> >>>
> >>> If the process is an oom victim then _all_ threads are so as well
> >>> because that is the address space property. And we already do check that
> >>> before reaching oom_badness IIRC. So what is the actual problem you are
> >>> trying to solve here?
> >>
> >> I'm talking about behavioral change after tsk became an OOM victim.
> >>
> >> If tsk->signal->oom_mm != NULL, we have to wait for MMF_OOM_SKIP even if
> >> tsk->mm == NULL. Otherwise, the OOM killer selects next OOM victim as soon as
> >> oom_unkillable_task() returned true because has_intersects_mems_allowed() returned
> >> false because mempolicy_nodemask_intersects() returned false because all thread's
> >> mm became NULL (despite tsk->signal->oom_mm != NULL).
> > 
> > OK, I finally got your point. It was not clear that you are referring to
> > the code _after_ the patch you are proposing. You are indeed right that
> > this would have a side effect that an additional victim could be
> > selected even though the current process hasn't terminated yet. Sigh,
> > another example how the whole thing is subtle so I retract my Ack and
> > request a real life example of where this matters before we think about
> > a proper fix and make the code even more complex.
> > 
> 
> Instead of checking for mm != NULL, can we move mpol_put_task_policy() from
> do_exit() to __put_task_struct() ? That change will (if it is safe to do)
> prevent exited threads from setting mempolicy = NULL (and confusing
> mempolicy_nodemask_intersects() due to mempolicy == NULL).

I am sorry but I would have to study it much more and I am not convinced
the time spent on it would be well spent.

-- 
Michal Hocko
SUSE Labs


  reply	other threads:[~2019-07-01 14:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-29 11:24 [PATCH] mm: mempolicy: don't select exited threads as OOM victims Tetsuo Handa
2019-07-01 11:17 ` Michal Hocko
2019-07-01 13:04   ` Tetsuo Handa
2019-07-01 13:17     ` Michal Hocko
2019-07-01 13:38       ` Tetsuo Handa
2019-07-01 13:48         ` Michal Hocko
2019-07-01 13:56           ` Tetsuo Handa
2019-07-01 14:04             ` Michal Hocko [this message]
2019-07-01 14:16               ` Michal Hocko
2019-07-02 13:19                 ` Tetsuo Handa
2019-07-02 13:51                   ` Michal Hocko
2019-07-02 21:26                     ` Tetsuo Handa
2019-07-03  7:06                       ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190701140434.GA6376@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=shakeelb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.