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: 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: Sat, 6 Feb 2016 09:37:58 +0100	[thread overview]
Message-ID: <20160206083757.GB25220@dhcp22.suse.cz> (raw)
In-Reply-To: <201602061454.GDG43774.LSHtOOMFOFVJQF@I-love.SAKURA.ne.jp>

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.
 
> Then, where is the guarantee that victim1 (first mm!=NULL thread in memcg1
> which got TIF_MEMDIE) is not waiting at down_read(&victim2->mm->mmap_sem)
> when victim2 (first mm!=NULL thread in memcg2 which got TIF_MEMDIE) is
> waiting at down_write(&victim2->mm->mmap_sem)

All threads/processes sharing the same mm are in fact in the same memory
cgroup. That is the reason we have owner in the task_struct

> or both victim1 and victim2
> are waiting on a lock somewhere in memory reclaim path (e.g.
> mutex_lock(&inode->i_mutex))?

Such waiting has to make a forward progress at some point in time
because the lock itself cannot be deadlocked by the memcg OOM context.


-- 
Michal Hocko
SUSE Labs

WARNING: multiple messages have this Message-ID (diff)
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: Sat, 6 Feb 2016 09:37:58 +0100	[thread overview]
Message-ID: <20160206083757.GB25220@dhcp22.suse.cz> (raw)
In-Reply-To: <201602061454.GDG43774.LSHtOOMFOFVJQF@I-love.SAKURA.ne.jp>

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.
 
> Then, where is the guarantee that victim1 (first mm!=NULL thread in memcg1
> which got TIF_MEMDIE) is not waiting at down_read(&victim2->mm->mmap_sem)
> when victim2 (first mm!=NULL thread in memcg2 which got TIF_MEMDIE) is
> waiting at down_write(&victim2->mm->mmap_sem)

All threads/processes sharing the same mm are in fact in the same memory
cgroup. That is the reason we have owner in the task_struct

> or both victim1 and victim2
> are waiting on a lock somewhere in memory reclaim path (e.g.
> mutex_lock(&inode->i_mutex))?

Such waiting has to make a forward progress at some point in time
because the lock itself cannot be deadlocked by the memcg OOM context.


-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2016-02-06  8:38 UTC|newest]

Thread overview: 112+ 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 ` Michal Hocko
2016-02-03 13:13 ` [PATCH 1/5] mm, oom: introduce oom reaper Michal Hocko
2016-02-03 13:13   ` Michal Hocko
2016-02-03 23:48   ` David Rientjes
2016-02-03 23:48     ` David Rientjes
2016-02-04  6:41     ` Michal Hocko
2016-02-04  6:41       ` Michal Hocko
2016-02-06 13:22   ` Tetsuo Handa
2016-02-06 13:22     ` Tetsuo Handa
2016-02-15 20:50     ` Michal Hocko
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 13:13   ` Michal Hocko
2016-02-03 23:57   ` David Rientjes
2016-02-03 23:57     ` David Rientjes
2016-02-23  1:36   ` David Rientjes
2016-02-23  1:36     ` David Rientjes
2016-02-23 13:21     ` Michal Hocko
2016-02-23 13:21       ` Michal Hocko
2016-02-29  3:19       ` Hugh Dickins
2016-02-29  3:19         ` Hugh Dickins
2016-02-29 13:41         ` Michal Hocko
2016-02-29 13:41           ` Michal Hocko
2016-03-08 13:40           ` Michal Hocko
2016-03-08 13:40             ` Michal Hocko
2016-03-08 20:07             ` Hugh Dickins
2016-03-08 20:07               ` Hugh Dickins
2016-03-09  8:26               ` Michal Hocko
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-03 13:13   ` Michal Hocko
2016-02-04 14:22   ` Tetsuo Handa
2016-02-04 14:22     ` Tetsuo Handa
2016-02-04 14:43     ` Michal Hocko
2016-02-04 14:43       ` Michal Hocko
2016-02-04 15:08       ` Tetsuo Handa
2016-02-04 15:08         ` Tetsuo Handa
2016-02-04 16:31         ` Michal Hocko
2016-02-04 16:31           ` Michal Hocko
2016-02-05 11:14           ` Tetsuo Handa
2016-02-05 11:14             ` Tetsuo Handa
2016-02-06  8:30             ` Michal Hocko
2016-02-06  8:30               ` Michal Hocko
2016-02-06 11:23               ` Tetsuo Handa
2016-02-06 11:23                 ` Tetsuo Handa
2016-02-15 20:47                 ` Michal Hocko
2016-02-15 20:47                   ` Michal Hocko
2016-02-06  6:45       ` Michal Hocko
2016-02-06  6:45         ` Michal Hocko
2016-02-06 14:33         ` Tetsuo Handa
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-15 20:40             ` Michal Hocko
2016-02-25 11:28   ` [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Tetsuo Handa
2016-02-25 11:31     ` Tetsuo Handa
2016-02-25 14:16     ` Michal Hocko
2016-02-03 13:13 ` [PATCH 4/5] mm, oom_reaper: report success/failure Michal Hocko
2016-02-03 13:13   ` Michal Hocko
2016-02-03 23:10   ` David Rientjes
2016-02-03 23:10     ` David Rientjes
2016-02-04  6:46     ` Michal Hocko
2016-02-04  6:46       ` Michal Hocko
2016-02-04 22:31       ` David Rientjes
2016-02-04 22:31         ` David Rientjes
2016-02-05  9:26         ` Michal Hocko
2016-02-05  9:26           ` Michal Hocko
2016-02-06  6:34           ` 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-03 13:14   ` Michal Hocko
2016-02-04 10:49   ` Tetsuo Handa
2016-02-04 10:49     ` Tetsuo Handa
2016-02-04 14:53     ` Michal Hocko
2016-02-04 14:53       ` Michal Hocko
2016-02-06  5:54       ` Tetsuo Handa
2016-02-06  5:54         ` Tetsuo Handa
2016-02-06  8:37         ` Michal Hocko [this message]
2016-02-06  8:37           ` Michal Hocko
2016-02-06 15:33           ` Tetsuo Handa
2016-02-06 15:33             ` Tetsuo Handa
2016-02-15 20:15             ` Michal Hocko
2016-02-15 20:15               ` Michal Hocko
2016-02-16 11:11               ` Tetsuo Handa
2016-02-16 11:11                 ` Tetsuo Handa
2016-02-16 15:53                 ` Michal Hocko
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  9:48     ` Michal Hocko
2016-02-17 10:41     ` Tetsuo Handa
2016-02-17 10:41       ` Tetsuo Handa
2016-02-17 11:33       ` Michal Hocko
2016-02-17 11:33         ` Michal Hocko
2016-02-19 18:34     ` 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-20  2:32         ` Tetsuo Handa
2016-02-22  9:41         ` Michal Hocko
2016-02-22  9:41           ` Michal Hocko
2016-02-29  1:26           ` Tetsuo Handa
2016-03-15 11:15           ` Tetsuo Handa
2016-03-15 11:43             ` Michal Hocko
2016-03-15 11:50               ` Michal Hocko
2016-03-16 11:16                 ` Tetsuo Handa
2016-03-17 10:49                   ` Tetsuo Handa
2016-03-17 12:17                     ` Michal Hocko
2016-03-17 13:00                       ` Tetsuo Handa
2016-03-17 13:23                         ` Michal Hocko
2016-03-17 14:34                           ` Tetsuo Handa
2016-03-17 14:54                             ` Michal Hocko
2016-03-17 15:20                               ` Tetsuo Handa
2016-03-17 12:14                   ` 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=20160206083757.GB25220@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 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.