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=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 1411DC54FCB for ; Fri, 24 Apr 2020 13:14:55 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C9C892084D for ; Fri, 24 Apr 2020 13:14:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=cmpxchg-org.20150623.gappssmtp.com header.i=@cmpxchg-org.20150623.gappssmtp.com header.b="sI9lU2rw" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C9C892084D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 643B48E0005; Fri, 24 Apr 2020 09:14:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5CC888E0003; Fri, 24 Apr 2020 09:14:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 494B98E0005; Fri, 24 Apr 2020 09:14:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0059.hostedemail.com [216.40.44.59]) by kanga.kvack.org (Postfix) with ESMTP id 2E8338E0003 for ; Fri, 24 Apr 2020 09:14:54 -0400 (EDT) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id DA0192488 for ; Fri, 24 Apr 2020 13:14:53 +0000 (UTC) X-FDA: 76742793666.24.birth65_d96fcc645832 X-HE-Tag: birth65_d96fcc645832 X-Filterd-Recvd-Size: 7706 Received: from mail-qv1-f68.google.com (mail-qv1-f68.google.com [209.85.219.68]) by imf14.hostedemail.com (Postfix) with ESMTP for ; Fri, 24 Apr 2020 13:14:53 +0000 (UTC) Received: by mail-qv1-f68.google.com with SMTP id 59so2491958qva.13 for ; Fri, 24 Apr 2020 06:14:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Mp6P7EuowXgf1Z05AVHM5X9/T2fvGq2V0AUIWl56Bfc=; b=sI9lU2rwmoOgQb4sOWPMx6/2JwNm8o6yusvDcM5jFn65xHqx1AVtLstARmFCCi785q ciie5Q/3FVP5a09qc6LjdzXeBx/h0JcAfwN5KrN0uDBY7IJ9T7PadA7MtakDn/ptURUG e1JRgF3OothN+HKLtVW4vFtbaJSbsApMBao/AgMgdc+vLUsIFAS8m2k6p5F6r8yQcFQD tfcA2GoN+PIjquTuPyUPMswEr/JdvQdoAvnWYEVMV0Bwo1uW17IqXdAivknRTGabND9/ Tjc+nE7V6fX9Rc1DuUbsAUjWQbaS4O5kYbiIbqkL/2+T7eBCT/KieV94Z0Hsjp6qLQ9f Apqg== 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=Mp6P7EuowXgf1Z05AVHM5X9/T2fvGq2V0AUIWl56Bfc=; b=mqdiFl1XShGU0jpkBsWl/W578tEMBjvu+9yEzpLCYhu4/Ww09VEKyeuUsJCtNpN7lI aujBRnMSgnKId7zceh1QOLHZXVMJXjk8vYx2bynCByVTjxOgXo298GiVGiU6FS6vYrTd D374VhFl4pvmQgLiRtGmETEtzof22DOIDy49+sVxRxVxnHTBDNmrYvR5xL/iZa7j/k4r e23odXJvkLTY7ayziBUBT77Lbhu+d0FQum8GYNYrYn3KEeo0J/0UkgDU7whQfUgvSOIu qaUuEs3k6FuY5yKwO5p3MFi4SKt3udL/DN4rmvjjjwGNqQ/83zr3v4//3F+Gp21Dl/sZ HHrA== X-Gm-Message-State: AGi0PuaAb/1k2YnoSNZti/pIcyIWnGvbDcY6LCXeqpZBn9UlpquwoefN MWdShC5Ro1v71I08A4pF/Q8ieA== X-Google-Smtp-Source: APiQypK8s2Ta3EVuQQRTzavHi6y2BDX8/R3BppjjGl+RVv9oXfLDBkmlpV0kzCcV9rFLr/QZdupKOQ== X-Received: by 2002:a0c:e8c2:: with SMTP id m2mr8747891qvo.24.1587734092456; Fri, 24 Apr 2020 06:14:52 -0700 (PDT) Received: from localhost ([2620:10d:c091:480::921]) by smtp.gmail.com with ESMTPSA id u126sm3666208qkh.66.2020.04.24.06.14.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Apr 2020 06:14:51 -0700 (PDT) Date: Fri, 24 Apr 2020 09:14:50 -0400 From: Johannes Weiner To: Yafang Shao Cc: akpm@linux-foundation.org, mhocko@kernel.org, vdavydov.dev@gmail.com, linux-mm@kvack.org, Chris Down , Roman Gushchin , stable@vger.kernel.org Subject: Re: [PATCH] mm, memcg: fix wrong mem cgroup protection Message-ID: <20200424131450.GA495720@cmpxchg.org> References: <20200423061629.24185-1-laoar.shao@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200423061629.24185-1-laoar.shao@gmail.com> 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 Thu, Apr 23, 2020 at 02:16:29AM -0400, Yafang Shao wrote: > This patch is an improvement of a previous version[1], as the previous > version is not easy to understand. > This issue persists in the newest kernel, I have to resend the fix. As > the implementation is changed, I drop Roman's ack from the previous > version. Now that I understand the problem, I much prefer the previous version. diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 745697906ce3..2bf91ae1e640 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -6332,8 +6332,19 @@ enum mem_cgroup_protection mem_cgroup_protected(struct mem_cgroup *root, if (!root) root = root_mem_cgroup; - if (memcg == root) + if (memcg == root) { + /* + * The cgroup is the reclaim root in this reclaim + * cycle, and therefore not protected. But it may have + * stale effective protection values from previous + * cycles in which it was not the reclaim root - for + * example, global reclaim followed by limit reclaim. + * Reset these values for mem_cgroup_protection(). + */ + memcg->memory.emin = 0; + memcg->memory.elow = 0; return MEMCG_PROT_NONE; + } usage = page_counter_read(&memcg->memory); if (!usage) > Here's the explanation of this issue. > memory.{low,min} won't take effect if the to-be-reclaimed memcg is the > sc->target_mem_cgroup, that can also be proved by the implementation in > mem_cgroup_protected(), see bellow, > mem_cgroup_protected > if (memcg == root) [2] > return MEMCG_PROT_NONE; > > But this rule is ignored in mem_cgroup_protection(), which will read > memory.{emin, elow} as the protection whatever the memcg is. > > How would this issue happen? > Because in mem_cgroup_protected() we forget to clear the > memory.{emin, elow} if the memcg is target_mem_cgroup [2]. > > An example to illustrate this issue. > root_mem_cgroup > / > A memory.max: 1024M > memory.min: 512M > memory.current: 800M ('current' must be greater than 'min') > Once kswapd starts to reclaim memcg A, it assigns 512M to memory.emin of A. > Then kswapd stops. > As a result of it, the memory values of A will be, > root_mem_cgroup > / > A memory.max: 1024M > memory.min: 512M > memory.current: 512M (approximately) > memory.emin: 512M > > Then a new workload starts to run in memcg A, and it will trigger memcg > relcaim in A soon. As memcg A is the target_mem_cgroup of this > reclaimer, so it return directly without touching memory.{emin, elow}.[2] > The memory values of A will be, > root_mem_cgroup > / > A memory.max: 1024M > memory.min: 512M > memory.current: 1024M (approximately) > memory.emin: 512M > Then this memory.emin will be used in mem_cgroup_protection() to get the > scan count, which is obvoiusly a wrong scan count. > > [1]. https://lore.kernel.org/linux-mm/20200216145249.6900-1-laoar.shao@gmail.com/ > > Fixes: 9783aa9917f8 ("mm, memcg: proportional memory.{low,min} reclaim") > Cc: Chris Down > Cc: Roman Gushchin > Cc: stable@vger.kernel.org > Signed-off-by: Yafang Shao As others have noted, it's fairly hard to understand the problem from the above changelog. How about the following: A cgroup can have both memory protection and a memory limit to isolate it from its siblings in both directions - for example, to prevent it from being shrunk below 2G under high pressure from outside, but also from growing beyond 4G under low pressure. 9783aa9917f8 ("mm, memcg: proportional memory.{low,min} reclaim") implemented proportional scan pressure so that multiple siblings in excess of their protection settings don't get reclaimed equally but instead in accordance to their unprotected portion. During limit reclaim, this proportionality shouldn't apply of course: there is no competition, all pressure is from within the cgroup and should be applied as such. Reclaim should operate at full efficiency. However, mem_cgroup_protected() never expected anybody to look at the effective protection values when it indicated that the cgroup is above its protection. As a result, a query during limit reclaim may return stale protection values that were calculated by a previous reclaim cycle in which the cgroup did have siblings. When this happens, reclaim is unnecessarily hesitant and potentially slow to meet the desired limit. In theory this could lead to premature OOM kills, although it's not obvious this has occurred in practice.