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 15:48:59 +0200	[thread overview]
Message-ID: <20190701134859.GZ6376@dhcp22.suse.cz> (raw)
In-Reply-To: <ecc63818-701f-403e-4d15-08c3f8aea8fb@i-love.sakura.ne.jp>

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.

-- 
Michal Hocko
SUSE Labs


  reply	other threads:[~2019-07-01 13:49 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 [this message]
2019-07-01 13:56           ` Tetsuo Handa
2019-07-01 14:04             ` Michal Hocko
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=20190701134859.GZ6376@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.