From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F1B0AC10DCE for ; Tue, 10 Mar 2020 22:19:42 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id A0E0A208E4 for ; Tue, 10 Mar 2020 22:19:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A0E0A208E4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 4B29F6B0003; Tue, 10 Mar 2020 18:19:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 461C96B0006; Tue, 10 Mar 2020 18:19:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 39F876B0007; Tue, 10 Mar 2020 18:19:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0126.hostedemail.com [216.40.44.126]) by kanga.kvack.org (Postfix) with ESMTP id 20FDD6B0003 for ; Tue, 10 Mar 2020 18:19:42 -0400 (EDT) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id E4CEE7599 for ; Tue, 10 Mar 2020 22:19:41 +0000 (UTC) X-FDA: 76580870562.18.dust26_71bb510d6ad50 X-HE-Tag: dust26_71bb510d6ad50 X-Filterd-Recvd-Size: 7320 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by imf11.hostedemail.com (Postfix) with ESMTP for ; Tue, 10 Mar 2020 22:19:41 +0000 (UTC) Received: by mail-wr1-f68.google.com with SMTP id t11so63085wrw.5 for ; Tue, 10 Mar 2020 15:19:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=8YYMvcq44BhLcSajpsg/HNjGZ5S6odZLEvG4zvpCcrA=; b=ACQZCsvpDvAsaVOoJ9La1iwhGeD5T6ioQrMiY/vQd3thNtSgVSeQSvdFNrh/Qeey8U OPxN75Retp8MHB623AeOoHEQ0zn9PV35nOsKbQIc50OgHVKTvlcp12qb2RHq7cRFU3kn jBkuJqXcJRAovKqX1u8vUIGZWNfWKlQbj9VJQCi8eQP/SFF8dUgc1K4rrkASushZK4E3 BuPX90Ka99J6kHCaxKI2VyHw7cclezBbGQpy0t2RZlqCL4yETLXTyIKL71h28ALj1EgP /9jKKTlATjro96Dgt5l411xm73caWb1O9obHPPZzfeKb93uEto/wBmIJgUp0pbD40Fuo ZkTg== X-Gm-Message-State: ANhLgQ0VyikKagkdWPDbQd0rfoZoAxL9FAolklM8202tuKLqojaEnkqj CH69RUfKOV0jRk8AHwDXias= X-Google-Smtp-Source: ADFU+vs6ekMje5xysr+W5uQbBCABjFxaf+IU/6zPEo0KmG9ptNpnuYl5xtK/J+siEw/iWMvcowthnQ== X-Received: by 2002:a5d:6591:: with SMTP id q17mr56234wru.22.1583878780380; Tue, 10 Mar 2020 15:19:40 -0700 (PDT) Received: from localhost (ip-37-188-253-35.eurotel.cz. [37.188.253.35]) by smtp.gmail.com with ESMTPSA id n5sm9583179wrx.80.2020.03.10.15.19.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Mar 2020 15:19:39 -0700 (PDT) Date: Tue, 10 Mar 2020 23:19:38 +0100 From: Michal Hocko To: David Rientjes Cc: Andrew Morton , Vlastimil Babka , linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [patch] mm, oom: make a last minute check to prevent unnecessary memcg oom kills Message-ID: <20200310221938.GF8447@dhcp22.suse.cz> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue 10-03-20 14:55:50, David Rientjes wrote: > Killing a user process as a result of hitting memcg limits is a serious > decision that is unfortunately needed only when no forward progress in > reclaiming memory can be made. > > Deciding the appropriate oom victim can take a sufficient amount of time > that allows another process that is exiting to actually uncharge to the > same memcg hierarchy and prevent unnecessarily killing user processes. > > An example is to prevent *multiple* unnecessary oom kills on a system > with two cores where the oom kill occurs when there is an abundance of > free memory available: > > Memory cgroup out of memory: Killed process 628 (repro) total-vm:41944kB, anon-rss:40888kB, file-rss:496kB, shmem-rss:0kB, UID:0 pgtables:116kB oom_score_adj:0 > > repro invoked oom-killer: gfp_mask=0xcc0(GFP_KERNEL), order=0, oom_score_adj=0 > CPU: 1 PID: 629 Comm: repro Not tainted 5.6.0-rc5+ #130 > Call Trace: > dump_stack+0x78/0xb6 > dump_header+0x55/0x240 > oom_kill_process+0xc5/0x170 > out_of_memory+0x305/0x4a0 > try_charge+0x77b/0xac0 > mem_cgroup_try_charge+0x10a/0x220 > mem_cgroup_try_charge_delay+0x1e/0x40 > handle_mm_fault+0xdf2/0x15f0 > do_user_addr_fault+0x21f/0x420 > async_page_fault+0x2f/0x40 > memory: usage 61336kB, limit 102400kB, failcnt 74 > > Notice the second memcg oom kill shows usage is >40MB below its limit of > 100MB but a process is still unnecessarily killed because the decision has > already been made to oom kill by calling out_of_memory() before the > initial victim had a chance to uncharge its memory. Could you be more specific about the specific workload please? > Make a last minute check to determine if an oom kill is really needed to > prevent unnecessary oom killing. I really see no reason why the memcg oom should behave differently from the global case. In both cases there will be a point of no return. Where-ever it is done it will be racy and the oom victim selection will play the race window role. There is simply no way around that without making the whole thing completely synchronous. This all looks like a micro optimization and I would really like to see a relevant real world usecase presented before new special casing is added. > > Cc: Vlastimil Babka > Cc: Michal Hocko > Cc: stable@vger.kernel.org > Signed-off-by: David Rientjes > --- > include/linux/memcontrol.h | 7 +++++++ > mm/memcontrol.c | 2 +- > mm/oom_kill.c | 16 +++++++++++++--- > 3 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -445,6 +445,8 @@ void mem_cgroup_iter_break(struct mem_cgroup *, struct mem_cgroup *); > int mem_cgroup_scan_tasks(struct mem_cgroup *, > int (*)(struct task_struct *, void *), void *); > > +unsigned long mem_cgroup_margin(struct mem_cgroup *memcg); > + > static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) > { > if (mem_cgroup_disabled()) > @@ -945,6 +947,11 @@ static inline int mem_cgroup_scan_tasks(struct mem_cgroup *memcg, > return 0; > } > > +static inline unsigned long mem_cgroup_margin(struct mem_cgroup *memcg) > +{ > + return 0; > +} > + > static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) > { > return 0; > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1286,7 +1286,7 @@ void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru, > * Returns the maximum amount of memory @mem can be charged with, in > * pages. > */ > -static unsigned long mem_cgroup_margin(struct mem_cgroup *memcg) > +unsigned long mem_cgroup_margin(struct mem_cgroup *memcg) > { > unsigned long margin = 0; > unsigned long count; > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -972,9 +972,6 @@ static void oom_kill_process(struct oom_control *oc, const char *message) > } > task_unlock(victim); > > - if (__ratelimit(&oom_rs)) > - dump_header(oc, victim); > - > /* > * Do we need to kill the entire memory cgroup? > * Or even one of the ancestor memory cgroups? > @@ -982,6 +979,19 @@ static void oom_kill_process(struct oom_control *oc, const char *message) > */ > oom_group = mem_cgroup_get_oom_group(victim, oc->memcg); > > + if (is_memcg_oom(oc)) { > + cond_resched(); > + > + /* One last check: do we *really* need to kill? */ > + if (mem_cgroup_margin(oc->memcg) >= (1 << oc->order)) { > + put_task_struct(victim); > + return; > + } > + } > + > + if (__ratelimit(&oom_rs)) > + dump_header(oc, victim); > + > __oom_kill_process(victim, message); > > /* -- Michal Hocko SUSE Labs