LKML Archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: akpm@linux-foundation.org, rientjes@google.com, mgorman@suse.de,
	oleg@redhat.com, torvalds@linux-foundation.org, hughd@google.com,
	andrea@kernel.org, riel@redhat.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing
Date: Mon, 15 Feb 2016 21:15:36 +0100	[thread overview]
Message-ID: <20160215201535.GB9223@dhcp22.suse.cz> (raw)
In-Reply-To: <201602070033.GFC13307.MOJQtFHOFOVLFS@I-love.SAKURA.ne.jp>

On Sun 07-02-16 00:33:38, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Sat 06-02-16 14:54:24, Tetsuo Handa wrote:
> > > Michal Hocko wrote:
> > > > > But if we consider non system-wide OOM events, it is not very unlikely to hit
> > > > > this race. This queue is useful for situations where memcg1 and memcg2 hit
> > > > > memcg OOM at the same time and victim1 in memcg1 cannot terminate immediately.
> > > > 
> > > > This can happen of course but the likelihood is _much_ smaller without
> > > > the global OOM because the memcg OOM killer is invoked from a lockless
> > > > context so the oom context cannot block the victim to proceed.
> > > 
> > > Suppose mem_cgroup_out_of_memory() is called from a lockless context via
> > > mem_cgroup_oom_synchronize() called from pagefault_out_of_memory(), that
> > > "lockless" is talking about only current thread, doesn't it?
> > 
> > Yes and you need the OOM context to sit on the same lock as the victim
> > to form a deadlock. So while the victim might be blocked somewhere it is
> > much less likely it would be deadlocked.
> > 
> > > Since oom_kill_process() sets TIF_MEMDIE on first mm!=NULL thread of a
> > > victim process, it is possible that non-first mm!=NULL thread triggers
> > > pagefault_out_of_memory() and first mm!=NULL thread gets TIF_MEMDIE,
> > > isn't it?
> > 
> > I got lost here completely. Maybe it is your usage of thread terminology
> > again.
> 
> I'm using "process" == "thread group" which contains at least one "thread",
> and "thread" == "struct task_struct".
> My assumption is
> 
>    (1) app1 process has two threads named app1t1 and app1t2
>    (2) app2 process has two threads named app2t1 and app2t2
>    (3) app1t1->mm == app1t2->mm != NULL and app2t1->mm == app2t2->mm != NULL
>    (4) app1 is in memcg1 and app2 is in memcg2
> 
> and sequence is
> 
>    (1) app1t2 triggers pagefault_out_of_memory()
>    (2) app1t2 calls mem_cgroup_out_of_memory() via mem_cgroup_oom_synchronize()
>    (3) oom_scan_process_thread() selects app1 as an OOM victim process
>    (4) find_lock_task_mm() selects app1t1 as an OOM victim thread
>    (5) app1t1 gets TIF_MEMDIE

OK so we have a victim in memcg1 and app1t2 will get to do_exit right away
because we are in the page fault path...

>    (6) app2t2 triggers pagefault_out_of_memory()
>    (7) app2t2 calls mem_cgroup_out_of_memory() via mem_cgroup_oom_synchronize()
>    (8) oom_scan_process_thread() selects app2 as an OOM victim process
>    (9) find_lock_task_mm() selects app2t1 as an OOM victim thread
>    (10) app2t1 gets TIF_MEMDIE
> 
> .
> 
> I'm talking about situation where app1t1 is blocked at down_write(&app1t1->mm->mmap_sem)
> because somebody else is already waiting at down_read(&app1t1->mm->mmap_sem) or is
> doing memory allocation between down_read(&app1t1->mm->mmap_sem) and
> up_read(&app1t1->mm->mmap_sem).

Unless we are under global OOM then this doesn't matter much because the
allocation request should succeed at some point in time and memcg
charges are bypassed for tasks with pending fatal signals. So we can
make a forward progress.

> In this case, this [PATCH 5/5] helps the OOM reaper to reap app2t1->mm
> after giving up waiting for down_read(&app1t1->mm->mmap_sem) to
> succeed.

Why does that matter at all?

-- 
Michal Hocko
SUSE Labs

  reply	other threads:[~2016-02-15 20:15 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-03 13:13 [PATCH 0/5] oom reaper v5 Michal Hocko
2016-02-03 13:13 ` [PATCH 1/5] mm, oom: introduce oom reaper Michal Hocko
2016-02-03 23:48   ` David Rientjes
2016-02-04  6:41     ` Michal Hocko
2016-02-06 13:22   ` Tetsuo Handa
2016-02-15 20:50     ` Michal Hocko
2016-02-03 13:13 ` [PATCH 2/5] oom reaper: handle mlocked pages Michal Hocko
2016-02-03 23:57   ` David Rientjes
2016-02-23  1:36   ` David Rientjes
2016-02-23 13:21     ` Michal Hocko
2016-02-29  3:19       ` Hugh Dickins
2016-02-29 13:41         ` Michal Hocko
2016-03-08 13:40           ` Michal Hocko
2016-03-08 20:07             ` Hugh Dickins
2016-03-09  8:26               ` Michal Hocko
2016-02-03 13:13 ` [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Michal Hocko
2016-02-04 14:22   ` Tetsuo Handa
2016-02-04 14:43     ` Michal Hocko
2016-02-04 15:08       ` Tetsuo Handa
2016-02-04 16:31         ` Michal Hocko
2016-02-05 11:14           ` Tetsuo Handa
2016-02-06  8:30             ` Michal Hocko
2016-02-06 11:23               ` Tetsuo Handa
2016-02-15 20:47                 ` Michal Hocko
2016-02-06  6:45       ` Michal Hocko
2016-02-06 14:33         ` Tetsuo Handa
2016-02-15 20:40           ` [PATCH 3.1/5] oom: make oom_reaper freezable Michal Hocko
2016-02-03 13:13 ` [PATCH 4/5] mm, oom_reaper: report success/failure Michal Hocko
2016-02-03 23:10   ` David Rientjes
2016-02-04  6:46     ` Michal Hocko
2016-02-04 22:31       ` David Rientjes
2016-02-05  9:26         ` Michal Hocko
2016-02-06  6:34           ` Michal Hocko
2016-02-03 13:14 ` [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing Michal Hocko
2016-02-04 10:49   ` Tetsuo Handa
2016-02-04 14:53     ` Michal Hocko
2016-02-06  5:54       ` Tetsuo Handa
2016-02-06  8:37         ` Michal Hocko
2016-02-06 15:33           ` Tetsuo Handa
2016-02-15 20:15             ` Michal Hocko [this message]
2016-02-16 11:11               ` Tetsuo Handa
2016-02-16 15:53                 ` Michal Hocko
2016-02-17  9:48   ` [PATCH 6/5] oom, oom_reaper: disable oom_reaper for Michal Hocko
2016-02-17 10:41     ` Tetsuo Handa
2016-02-17 11:33       ` Michal Hocko
2016-02-19 18:34     ` Michal Hocko
2016-02-20  2:32       ` [PATCH 6/5] oom, oom_reaper: disable oom_reaper for oom_kill_allocating_task Tetsuo Handa
2016-02-22  9:41         ` 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=20160215201535.GB9223@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=andrea@kernel.org \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=oleg@redhat.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).