From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@kernel.org>,
torvalds@linux-foundation.org
Cc: guro@fb.com, rientjes@google.com, hannes@cmpxchg.org,
vdavydov.dev@gmail.com, tj@kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH] mm,oom: Don't call schedule_timeout_killable() with oom_lock held.
Date: Thu, 31 May 2018 19:10:48 +0900 [thread overview]
Message-ID: <16eca862-5fa6-2333-8a81-94a2c2692758@i-love.sakura.ne.jp> (raw)
In-Reply-To: <20180529160700.dbc430ebbfac301335ac8cf4@linux-foundation.org>
On 2018/05/30 8:07, Andrew Morton wrote:
> On Tue, 29 May 2018 09:17:41 +0200 Michal Hocko <mhocko@kernel.org> wrote:
>
>>> I suggest applying
>>> this patch first, and then fix "mm, oom: cgroup-aware OOM killer" patch.
>>
>> Well, I hope the whole pile gets merged in the upcoming merge window
>> rather than stall even more.
>
> I'm more inclined to drop it all. David has identified significant
> shortcomings and I'm not seeing a way of addressing those shortcomings
> in a backward-compatible fashion. Therefore there is no way forward
> at present.
>
Can we apply my patch as-is first? My patch mitigates lockup regression
which should be able to be easily backported to stable kernels. We can
later evaluate whether moving the short sleep to should_reclaim_retry()
has negative impact.
Also we can eliminate the short sleep in Roman's patch before deciding
whether we can merge Roman's patchset in the upcoming merge window.
>From 4b356c742a3f1b720d5b709792fe68b25d800902 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 12 May 2018 12:27:52 +0900
Subject: [PATCH] mm,oom: Don't call schedule_timeout_killable() with oom_lock held.
When I was examining a bug which occurs under CPU + memory pressure, I
observed that a thread which called out_of_memory() can sleep for minutes
at schedule_timeout_killable(1) with oom_lock held when many threads are
doing direct reclaim.
The whole point of the sleep is give the OOM victim some time to exit.
But since commit 27ae357fa82be5ab ("mm, oom: fix concurrent munlock and
oom reaper unmap, v3") changed the OOM victim to wait for oom_lock in order
to close race window at exit_mmap(), the whole point of this sleep is lost
now. We need to make sure that the thread which called out_of_memory() will
release oom_lock shortly. Therefore, this patch brings the sleep to outside
of the OOM path.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Roman Gushchin <guro@fb.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Tejun Heo <tj@kernel.org>
---
mm/oom_kill.c | 38 +++++++++++++++++---------------------
mm/page_alloc.c | 7 ++++++-
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 8ba6cb8..23ce67f 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -479,6 +479,21 @@ bool process_shares_mm(struct task_struct *p, struct mm_struct *mm)
static struct task_struct *oom_reaper_list;
static DEFINE_SPINLOCK(oom_reaper_lock);
+/*
+ * We have to make sure not to cause premature new oom victim selection.
+ *
+ * __alloc_pages_may_oom() oom_reap_task_mm()/exit_mmap()
+ * mutex_trylock(&oom_lock)
+ * get_page_from_freelist(ALLOC_WMARK_HIGH) # fails
+ * unmap_page_range() # frees some memory
+ * set_bit(MMF_OOM_SKIP)
+ * out_of_memory()
+ * select_bad_process()
+ * test_bit(MMF_OOM_SKIP) # selects new oom victim
+ * mutex_unlock(&oom_lock)
+ *
+ * Therefore, the callers hold oom_lock when calling this function.
+ */
void __oom_reap_task_mm(struct mm_struct *mm)
{
struct vm_area_struct *vma;
@@ -523,20 +538,6 @@ static bool oom_reap_task_mm(struct task_struct *tsk, struct mm_struct *mm)
{
bool ret = true;
- /*
- * We have to make sure to not race with the victim exit path
- * and cause premature new oom victim selection:
- * oom_reap_task_mm exit_mm
- * mmget_not_zero
- * mmput
- * atomic_dec_and_test
- * exit_oom_victim
- * [...]
- * out_of_memory
- * select_bad_process
- * # no TIF_MEMDIE task selects new victim
- * unmap_page_range # frees some memory
- */
mutex_lock(&oom_lock);
if (!down_read_trylock(&mm->mmap_sem)) {
@@ -1077,15 +1078,9 @@ bool out_of_memory(struct oom_control *oc)
dump_header(oc, NULL);
panic("Out of memory and no killable processes...\n");
}
- if (oc->chosen && oc->chosen != (void *)-1UL) {
+ if (oc->chosen && oc->chosen != (void *)-1UL)
oom_kill_process(oc, !is_memcg_oom(oc) ? "Out of memory" :
"Memory cgroup out of memory");
- /*
- * Give the killed process a good chance to exit before trying
- * to allocate memory again.
- */
- schedule_timeout_killable(1);
- }
return !!oc->chosen;
}
@@ -1111,4 +1106,5 @@ void pagefault_out_of_memory(void)
return;
out_of_memory(&oc);
mutex_unlock(&oom_lock);
+ schedule_timeout_killable(1);
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 905db9d..458ed32 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3478,7 +3478,6 @@ void warn_alloc(gfp_t gfp_mask, nodemask_t *nodemask, const char *fmt, ...)
*/
if (!mutex_trylock(&oom_lock)) {
*did_some_progress = 1;
- schedule_timeout_uninterruptible(1);
return NULL;
}
@@ -4241,6 +4240,12 @@ bool gfp_pfmemalloc_allowed(gfp_t gfp_mask)
/* Retry as long as the OOM killer is making progress */
if (did_some_progress) {
no_progress_loops = 0;
+ /*
+ * This schedule_timeout_*() serves as a guaranteed sleep for
+ * PF_WQ_WORKER threads when __zone_watermark_ok() == false.
+ */
+ if (!tsk_is_oom_victim(current))
+ schedule_timeout_uninterruptible(1);
goto retry;
}
--
1.8.3.1
next prev parent reply other threads:[~2018-05-31 10:11 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-12 14:18 [PATCH] mm,oom: Don't call schedule_timeout_killable() with oom_lock held Tetsuo Handa
2018-05-15 9:16 ` Michal Hocko
2018-05-18 10:14 ` Tetsuo Handa
2018-05-18 12:20 ` Michal Hocko
2018-05-20 15:56 ` Tetsuo Handa
2018-05-22 6:18 ` Michal Hocko
2018-05-23 10:24 ` Tetsuo Handa
2018-05-23 11:57 ` Michal Hocko
2018-05-23 13:45 ` Tetsuo Handa
2018-05-23 14:56 ` Michal Hocko
2018-05-24 10:51 ` Tetsuo Handa
2018-05-24 11:50 ` Michal Hocko
2018-05-25 1:17 ` Tetsuo Handa
2018-05-25 8:31 ` Michal Hocko
2018-05-25 10:57 ` Tetsuo Handa
2018-05-25 11:42 ` Michal Hocko
2018-05-25 11:46 ` Tetsuo Handa
2018-05-28 12:43 ` Michal Hocko
2018-05-28 20:57 ` Tetsuo Handa
2018-05-29 7:17 ` Michal Hocko
2018-05-29 23:07 ` Andrew Morton
2018-05-31 10:10 ` Tetsuo Handa [this message]
2018-05-31 10:44 ` Michal Hocko
2018-05-31 15:23 ` Tetsuo Handa
2018-05-31 18:47 ` Michal Hocko
2018-06-01 1:21 ` Tetsuo Handa
2018-06-01 8:04 ` Michal Hocko
2018-06-01 15:28 ` Michal Hocko
2018-06-01 21:11 ` Andrew Morton
2018-06-04 7:04 ` Michal Hocko
2018-06-04 10:41 ` Tetsuo Handa
2018-06-04 11:22 ` Michal Hocko
2018-06-04 11:30 ` Tetsuo Handa
2018-06-06 9:02 ` David Rientjes
2018-06-06 13:37 ` Tetsuo Handa
2018-06-06 18:44 ` David Rientjes
2018-05-29 7:17 ` Michal Hocko
2018-05-29 8:16 ` Michal Hocko
2018-05-29 14:33 ` Tetsuo Handa
2018-05-29 17:18 ` Michal Hocko
2018-05-29 17:28 ` Michal Hocko
2018-05-31 16:31 ` [PATCH] mm, memcg, oom: fix pre-mature allocation failures kbuild test robot
-- strict thread matches above, loose matches on Subject: below --
2018-03-22 10:51 [PATCH] mm,oom: Don't call schedule_timeout_killable() with oom_lock held Tetsuo Handa
2018-03-22 11:45 ` Michal Hocko
2018-03-22 13:16 ` Tetsuo Handa
2018-01-22 13:46 Tetsuo Handa
2018-01-23 8:38 ` Michal Hocko
2018-01-23 12:07 ` Tetsuo Handa
2018-01-23 12:42 ` Michal Hocko
2018-01-24 13:28 ` Tetsuo Handa
2018-02-13 11:58 ` Tetsuo Handa
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=16eca862-5fa6-2333-8a81-94a2c2692758@i-love.sakura.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=akpm@linux-foundation.org \
--cc=guro@fb.com \
--cc=hannes@cmpxchg.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=rientjes@google.com \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=vdavydov.dev@gmail.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 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 NNTP newsgroup(s).