LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: syzbot <syzbot+bab151e82a4e973fa325@syzkaller.appspotmail.com>
Cc: cgroups@vger.kernel.org, dvyukov@google.com, hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	penguin-kernel@I-love.SAKURA.ne.jp,
	syzkaller-bugs@googlegroups.com, vdavydov.dev@gmail.com
Subject: Re: WARNING in try_charge
Date: Mon, 6 Aug 2018 21:45:53 +0200
Message-ID: <20180806194553.GH10003@dhcp22.suse.cz> (raw)
In-Reply-To: <0000000000006986c30572c90de3@google.com>

[CCing Greg - the email thread starts here
http://lkml.kernel.org/r/0000000000005e979605729c1564@google.com]

On Mon 06-08-18 12:12:02, syzbot wrote:
> Hello,
> 
> syzbot has tested the proposed patch and the reproducer did not trigger
> crash:

OK, this is reassuring. Btw Greg has pointed out this potential case
http://lkml.kernel.org/r/xr93in62jy8k.fsf@gthelen.svl.corp.google.com
but I simply didn't get what he meant. He was suggesting MMF_OOM_SKIP
but I didn't get why that matters. I didn't think about a race.

So how about this patch:
From 74d980f8d066d06ada657ebf9b586dbf5668ed26 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Mon, 6 Aug 2018 21:21:24 +0200
Subject: [PATCH] memcg, oom: be careful about races when warning about no
 reclaimable task

"memcg, oom: move out_of_memory back to the charge path" has added a
warning triggered when the oom killer cannot find any eligible task
and so there is no way to reclaim the oom memcg under its hard limit.
Further charges for such a memcg are forced and therefore the hard limit
isolation is weakened.

The current warning is however too eager to trigger  even when we are not
really hitting the above condition. Syzbot and Greg Thelen have noticed
that we can hit this condition even when there is still oom victim
pending. E.g. the following race is possible:

memcg has two tasks taskA, taskB.

CPU1 (taskA)			CPU2			CPU3 (taskB)
try_charge
  mem_cgroup_out_of_memory				try_charge
      select_bad_process(taskB)
      oom_kill_process		oom_reap_task
				# No real memory reaped
    				  			  mem_cgroup_out_of_memory
				# set taskB -> MMF_OOM_SKIP
  # retry charge
  mem_cgroup_out_of_memory
    oom_lock						    oom_lock
    select_bad_process(self)
    oom_kill_process(self)
    oom_unlock
							    # no eligible task

In fact syzbot test triggered this situation by placing multiple tasks
into a memcg with hard limit set to 0. So no task really had any memory
charged to the memcg

: Memory cgroup stats for /ile0: cache:0KB rss:0KB rss_huge:0KB shmem:0KB mapped_file:0KB dirty:0KB writeback:0KB swap:0KB inactive_anon:0KB active_anon:0KB inactive_file:0KB active_file:0KB unevictable:0KB
: Tasks state (memory values in pages):
: [  pid  ]   uid  tgid total_vm      rss pgtables_bytes swapents oom_score_adj name
: [   6569]     0  6562     9427        1    53248        0             0 syz-executor0
: [   6576]     0  6576     9426        0    61440        0             0 syz-executor6
: [   6578]     0  6578     9426      534    61440        0             0 syz-executor4
: [   6579]     0  6579     9426        0    57344        0             0 syz-executor5
: [   6582]     0  6582     9426        0    61440        0             0 syz-executor7
: [   6584]     0  6584     9426        0    57344        0             0 syz-executor1

so in principle there is indeed nothing reclaimable in this memcg and
this looks like a misconfiguration. On the other hand we can clearly
kill all those tasks so it is a bit early to warn and scare users. Do
that by checking that the current is the oom victim and bypass the
warning then. The victim is allowed to force charge and terminate to
release its temporal charge along the way.

Fixes: "memcg, oom: move out_of_memory back to the charge path"
Noticed-by: Greg Thelen <gthelen@google.com>
Reported-and-tested-by: syzbot+bab151e82a4e973fa325@syzkaller.appspotmail.com
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/memcontrol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4603ad75c9a9..1b6eed1bc404 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1703,7 +1703,8 @@ static enum oom_status mem_cgroup_oom(struct mem_cgroup *memcg, gfp_t mask, int
 		return OOM_ASYNC;
 	}
 
-	if (mem_cgroup_out_of_memory(memcg, mask, order))
+	if (mem_cgroup_out_of_memory(memcg, mask, order) ||
+			tsk_is_oom_victim(current))
 		return OOM_SUCCESS;
 
 	WARN(1,"Memory cgroup charge failed because of no reclaimable memory! "
-- 
2.18.0

-- 
Michal Hocko
SUSE Labs

  reply index

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-04 13:33 syzbot
2018-08-04 13:45 ` Tetsuo Handa
2018-08-05 11:33   ` Tetsuo Handa
2018-08-05  8:14 ` syzbot
2018-08-06  9:15 ` Michal Hocko
2018-08-06  9:30   ` Dmitry Vyukov
2018-08-06  9:48     ` Michal Hocko
2018-08-06 10:34       ` Dmitry Vyukov
2018-08-06 11:02         ` Michal Hocko
2018-08-06 11:57           ` Dmitry Vyukov
2018-08-06 14:21             ` Michal Hocko
2018-08-06 14:58               ` Dmitry Vyukov
2018-08-06 17:30                 ` Michal Hocko
2018-08-06 17:53                   ` Dmitry Vyukov
2018-08-06 15:07               ` Dmitry Vyukov
2018-08-06 15:31               ` Johannes Weiner
2018-08-06 10:39       ` Dmitry Vyukov
2018-08-06 10:47         ` Tetsuo Handa
2018-08-06 11:09           ` Michal Hocko
2018-08-06 11:27           ` syzbot
2018-08-06 11:32             ` Michal Hocko
2018-08-06 11:58               ` Dmitry Vyukov
2018-08-06 14:41               ` Tetsuo Handa
2018-08-06 14:58                 ` Michal Hocko
2018-08-06 15:12                   ` Tetsuo Handa
2018-08-06 14:54               ` David Howells
2018-08-06 15:04                 ` Tetsuo Handa
2018-08-06 11:00         ` syzbot
2018-08-06 15:32         ` Tetsuo Handa
2018-08-06 15:42           ` syzbot
2018-08-06 16:02             ` Tetsuo Handa
2018-08-06 17:44             ` Michal Hocko
2018-08-06 17:49               ` Dmitry Vyukov
2018-08-06 17:56               ` Michal Hocko
2018-08-06 18:13                 ` Michal Hocko
2018-08-06 18:23                   ` syzbot
2018-08-06 18:55                     ` Michal Hocko
2018-08-06 19:12                       ` syzbot
2018-08-06 19:45                         ` Michal Hocko [this message]
2018-08-06 19:46                           ` Michal Hocko
2018-08-07 11:18                       ` Dmitry Vyukov
2018-08-07 11:25                         ` Michal Hocko
2018-08-06 18:39                   ` Michal Hocko
2018-08-06 20:26                 ` Tetsuo Handa
2018-08-06 20:34                   ` Michal Hocko
2018-08-06 20:46                     ` Tetsuo Handa
2018-08-06 20:55                       ` Michal Hocko
2018-08-06 21:50                         ` Tetsuo Handa
2018-08-07 10:19                           ` Tetsuo Handa
2018-08-09 13:57 ` Tetsuo Handa
2018-08-09 15:07   ` Michal Hocko
2018-08-09 21:05     ` Tetsuo Handa
2018-08-09 15:34   ` Johannes Weiner

Reply instructions:

You may reply publically 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=20180806194553.GH10003@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=cgroups@vger.kernel.org \
    --cc=dvyukov@google.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=syzbot+bab151e82a4e973fa325@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox