Received: by 2002:ac0:a5a7:0:0:0:0:0 with SMTP id m36-v6csp478048imm; Tue, 7 Aug 2018 23:45:31 -0700 (PDT) X-Google-Smtp-Source: AA+uWPwbL74ZVhrfLlxB7yZv2y492Xmkdw+z2MBuGnyzLJkqO/Oij0rlFU9hk00B7UkKeElr4LKF X-Received: by 2002:a65:5144:: with SMTP id g4-v6mr1283982pgq.21.1533710730926; Tue, 07 Aug 2018 23:45:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1533710730; cv=none; d=google.com; s=arc-20160816; b=swDJ6IFfrj57HnIQGKFSYfWQLsYuyqNwmiXio875n8mYgVJET9I6ZEYgKXMhsZ5fr6 awLpX+4aZ4baaikt+tKrKSyJGYYcVYA5A+V5aoojdozxeHPQPRjtfl4GtSqGsznlMA9Q bRpDq78pvHo/ONx6lP79jiEY+ed/diV+kiTMkz7K0l9DMX3ReyDvFPN+ohjBl6Ht6F7P ZZtnIgwrzZjaL3BbarxKV6D3Bsop3yH+5ISeaQafMO61+0SvFdlVcGrO+Z2mP0Lrk2Rq EDh98zYYBIiw7DK/goUv6COHHJu42EVEvdoECNHTLn2jInx/rKnwi6YRNqqlHNJv59zu EBnQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:arc-authentication-results; bh=jda7MmsfFSU6ZZaAOD680T9YBDX41SR/Hn+y0YSrAf8=; b=GSuZL5aQs/UbbSmXLEf4/yjEtcYtFPjDKfwOwW+SFAinhHpfxH7mm2tD0tXXeWn+y+ bXZhZq+xUdiiqSpbh7IHPQ3vASYnRUeZGSfdvkiDVCkezOoJ3Jbl1Fk1zYzty5iLREpx osYvJB4AVjLIplVNiXBnZqyeqf7JeDxd22mWL2IRLHTSv91f70PuiZJ2DY2ce37DO/xr At1mFOZ/02NHOHY373foKZhgBQOwhSkfbGi+YHsN9K//wDrZ4pLYcfIy9KCsnDcnXf8e ve9875KPCluRPphDxyAm+43TFsqfCivCqFNlwKEAdPGVbi3bix1YsTN/Pb8dFc6u2LLv zRaw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w15-v6si3565476pga.30.2018.08.07.23.45.16; Tue, 07 Aug 2018 23:45:30 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727198AbeHHJCc (ORCPT + 99 others); Wed, 8 Aug 2018 05:02:32 -0400 Received: from mx2.suse.de ([195.135.220.15]:45248 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726957AbeHHJCc (ORCPT ); Wed, 8 Aug 2018 05:02:32 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 2D001AD85; Wed, 8 Aug 2018 06:44:15 +0000 (UTC) Date: Wed, 8 Aug 2018 08:44:14 +0200 From: Michal Hocko To: Johannes Weiner Cc: Andrew Morton , Vladimir Davydov , linux-mm@kvack.org, Greg Thelen , Tetsuo Handa , Dmitry Vyukov , LKML Subject: Re: [PATCH] memcg, oom: be careful about races when warning about no reclaimable task Message-ID: <20180808064414.GA27972@dhcp22.suse.cz> References: <20180807072553.14941-1-mhocko@kernel.org> <20180807200247.GA4251@cmpxchg.org> <20180807202332.GK10003@dhcp22.suse.cz> <20180807205425.GA5928@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180807205425.GA5928@cmpxchg.org> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue 07-08-18 16:54:25, Johannes Weiner wrote: > On Tue, Aug 07, 2018 at 10:23:32PM +0200, Michal Hocko wrote: > > On Tue 07-08-18 16:02:47, Johannes Weiner wrote: > > > On Tue, Aug 07, 2018 at 09:25:53AM +0200, Michal Hocko wrote: > > > > From: Michal Hocko > > > > > > > > "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[1] 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. > > > > > > > > [1] http://lkml.kernel.org/r/0000000000005e979605729c1564@google.com > > > > Fixes: "memcg, oom: move out_of_memory back to the charge path" > > > > Noticed-by: Greg Thelen > > > > Reported-and-tested-by: syzbot+bab151e82a4e973fa325@syzkaller.appspotmail.com > > > > Signed-off-by: Michal Hocko > > > > --- > > > > 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! " > > > > > > This is really ugly. :( > > > > > > If that check is only there to suppress the warning when the limit is > > > 0, this should really be a separate branch around the warning, with a > > > fat comment that this is a ridiculous cornercase, and not look like it > > > is an essential part of the memcg reclaim/oom process. > > > > I do not mind having it in a separate branch. Btw. this is not just about > > hard limit set to 0. Similar can happen anytime we are getting out of > > oom victims. The likelihood goes up with the remote memcg charging > > merged recently. > > What the global OOM killer does in that situation is dump the header > anyway: > > /* Found nothing?!?! Either we hang forever, or we panic. */ > if (!oc->chosen && !is_sysrq_oom(oc) && !is_memcg_oom(oc)) { > dump_header(oc, NULL); > panic("Out of memory and no killable processes...\n"); > } > > I think that would make sense here as well - without the panic, > obviously, but we can add our own pr_err() line following the header. > > That gives us the exact memory situation of the cgroup and who is > trying to allocate and from what context, but in a format that is > known to users without claiming right away that it's a kernel issue. I was considering doing that initially but then decided that warning is less noisy and still a good "let us know" trigger. It doesn't give us the whole picture which is obviously a downside but we would at least know that something is going south one have the trace to who that might be should this be a bug rather than a misconfiguration. But I do not mind doing dump_header as well. Care to send a patch? -- Michal Hocko SUSE Labs