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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C437C433FE for ; Wed, 23 Nov 2022 00:46:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2C8BD6B0078; Tue, 22 Nov 2022 19:46:29 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 278D26B007B; Tue, 22 Nov 2022 19:46:29 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 141308E0001; Tue, 22 Nov 2022 19:46:29 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 070226B0078 for ; Tue, 22 Nov 2022 19:46:29 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id B296C1A0AC5 for ; Wed, 23 Nov 2022 00:46:28 +0000 (UTC) X-FDA: 80162866056.10.605DE27 Received: from mail-il1-f178.google.com (mail-il1-f178.google.com [209.85.166.178]) by imf01.hostedemail.com (Postfix) with ESMTP id C1FCC40005 for ; Wed, 23 Nov 2022 00:46:26 +0000 (UTC) Received: by mail-il1-f178.google.com with SMTP id z9so7884190ilu.10 for ; Tue, 22 Nov 2022 16:46:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=HCqiKLLnoiFcJW128YgyhQV3aZ+j8zueWc+2Mjy7QDw=; b=P2OPgvc42GjVbzAFJtllZKLMFc61w1isQHKvEcP5yiMOWGleesqrhoLQIQhdsP6GDE 9ps4Tq2j7LR+SSAH64kH1RYuigJP8KjN/6AmU7YYZ4Lvn3Cgb7fOiqvN28u6CW/0K9Uf dKonoiT2zED/Kx4CTxRDpW9zGZ1mkvyg7q4vA/qp6GPpNkbX6Cehih0gRIhpbatSdfSZ cbaM9L0H8DQ65UZVwA/K5tVtRMmlA+zlRMVEyYy9P2cS3gw2Y4T2tetE5XchKLnLCX7u 2UaJGyeTgRsdUXq1dlrlP6FzM9aSYHLyomGWwfhRZpZSBWs8KfcmXVZz1TMnj6c+Zpcz 5EnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=HCqiKLLnoiFcJW128YgyhQV3aZ+j8zueWc+2Mjy7QDw=; b=JUFcivMbYOzq7IonIBbnH+Qd2AYIZ2sLiVC9wQ7Xw7WR4TX8RzoaHtpj+o0P4EyI0u Ht4JzMepmSq2KinVnKWeB2ktJ/Cp9m53bYwhzjLmwE6eMgXgOhkYcpzuj1oaSQrK9ouZ VZQ06xevTltqIq3hlTMp+nuqvmUJa1IOJb4AYbQrAmdCAjPFqU7HQR3D+ijqtZ0cieaC 6ShVKOFQkexMyfhiXZ0Hi3cuPZ7FpNGprQSkhiXBuM2AiqphljdNRhDb9fVOBoqjynCL NK+hDKC5QtJ9hj9Ny2Z7Gc16G0z4cW0b3lwsB516WTbaQYI92lBkr2YKCRQTuCc2xXU8 vU1w== X-Gm-Message-State: ANoB5pn/3pu5wgk3XmidgoreXtB+kSJJX3v9HgSwKkb4JbgTnKqEdG5s FFjezMft+vyjZG6r2eG892QL+LyQBDwPMJ35vuzs0g== X-Google-Smtp-Source: AA0mqf4L0GbX8I8q8w0YiGVmt4MtBul+hhQ7iVRKGIEZLiVPea/oqIJVTnMpmzrKMqVduKgfXyGBhocQot8JgF/gX/M= X-Received: by 2002:a92:7310:0:b0:302:571f:8d7f with SMTP id o16-20020a927310000000b00302571f8d7fmr11554477ilc.53.1669164385808; Tue, 22 Nov 2022 16:46:25 -0800 (PST) MIME-Version: 1.0 References: <20221122232721.2306102-1-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Tue, 22 Nov 2022 16:45:50 -0800 Message-ID: Subject: Re: [PATCH] mm: memcg: fix stale protection of reclaim target memcg To: Roman Gushchin Cc: Shakeel Butt , Johannes Weiner , Michal Hocko , Yu Zhao , Muchun Song , "Matthew Wilcox (Oracle)" , Vasily Averin , Vlastimil Babka , Chris Down , linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669164386; a=rsa-sha256; cv=none; b=faqReXt9cKvfiyN6zDwiH8lgFB4fL0tGqUMUKDCbfjsBvUS4KYMFp7QHzUeyfYjGlBGdOo VyAWGL1J+L8DC3Yk1jxrQLnxOjIzWSpVk8ZKrwQgIwbf8JGknOyaIY96FFXnpMc94kwQZ3 Y5Dgx0m93Dv55r+P8uVw1qtyqgJPBtY= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=P2OPgvc4; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of yosryahmed@google.com designates 209.85.166.178 as permitted sender) smtp.mailfrom=yosryahmed@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669164386; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HCqiKLLnoiFcJW128YgyhQV3aZ+j8zueWc+2Mjy7QDw=; b=H+UpwI17otwn3fQp6gwSAJxh1WMhqpvXlDwzzULC8ZxnjDnfxTwQQpj16f591nZcp8ibJ2 NQRqJ/2gbYvosXiLtFLY8CWhFQbmNKvcgsGN6ohRoLVv1bMCPnPmqu/N9JcATasl3AJo5n WnxPqPBPkWt2ezCBbvKvybh/yqbbq94= X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: C1FCC40005 X-Rspam-User: Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=P2OPgvc4; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of yosryahmed@google.com designates 209.85.166.178 as permitted sender) smtp.mailfrom=yosryahmed@google.com X-Stat-Signature: 3sus53d6i48i34dfj4hjff5a3yc76akd X-HE-Tag: 1669164386-949507 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000198, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Nov 22, 2022 at 4:37 PM Roman Gushchin wrote: > > On Tue, Nov 22, 2022 at 11:27:21PM +0000, Yosry Ahmed wrote: > > During reclaim, mem_cgroup_calculate_protection() is used to determine > > the effective protection (emin and elow) values of a memcg. The > > protection of the reclaim target is ignored, but we cannot set their > > effective protection to 0 due to a limitation of the current > > implementation (see comment in mem_cgroup_protection()). Instead, > > we leave their effective protection values unchaged, and later ignore it > > in mem_cgroup_protection(). > > > > However, mem_cgroup_protection() is called later in > > shrink_lruvec()->get_scan_count(), which is after the > > mem_cgroup_below_{min/low}() checks in shrink_node_memcgs(). As a > > result, the stale effective protection values of the target memcg may > > lead us to skip reclaiming from the target memcg entirely, before > > calling shrink_lruvec(). This can be even worse with recursive > > protection, where the stale target memcg protection can be higher than > > its standalone protection. > > > > An example where this can happen is as follows. Consider the following > > hierarchy with memory_recursiveprot: > > ROOT > > | > > A (memory.min = 50M) > > | > > B (memory.min = 10M, memory.high = 40M) > > > > Consider the following scenarion: > > - B has memory.current = 35M. > > - The system undergoes global reclaim (target memcg is NULL). > > - B will have an effective min of 50M (all of A's unclaimed protection). > > - B will not be reclaimed from. > > - Now allocate 10M more memory in B, pushing it above it's high limit. > > - The system undergoes memcg reclaim from B (target memcg is B) > > - In shrink_node_memcgs(), we call mem_cgroup_calculate_protection(), > > which immediately returns for B without doing anything, as B is the > > target memcg, relying on mem_cgroup_protection() to ignore B's stale > > effective min (still 50M). > > - Directly after mem_cgroup_calculate_protection(), we will call > > mem_cgroup_below_min(), which will read the stale effective min for B > > and skip it (instead of ignoring its protection as intended). In this > > case, it's really bad because we are not just considering B's > > standalone protection (10M), but we are reading a much higher stale > > protection (50M) which will cause us to not reclaim from B at all. > > > > This is an artifact of commit 45c7f7e1ef17 ("mm, memcg: decouple > > e{low,min} state mutations from protection checks") which made > > mem_cgroup_calculate_protection() only change the state without > > returning any value. Before that commit, we used to return > > MEMCG_PROT_NONE for the target memcg, which would cause us to skip the > > mem_cgroup_below_{min/low}() checks. After that commit we do not return > > anything and we end up checking the min & low effective protections for > > the target memcg, which are stale. > > > > Add mem_cgroup_ignore_protection() that checks if we are reclaiming from > > the target memcg, and call it in mem_cgroup_below_{min/low}() to ignore > > the stale protection of the target memcg. > > > > Fixes: 45c7f7e1ef17 ("mm, memcg: decouple e{low,min} state mutations from protection checks") > > Signed-off-by: Yosry Ahmed > > Great catch! > The fix looks good to me, only a couple of cosmetic suggestions. > > > --- > > include/linux/memcontrol.h | 33 +++++++++++++++++++++++++++------ > > mm/vmscan.c | 11 ++++++----- > > 2 files changed, 33 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > > index e1644a24009c..22c9c9f9c6b1 100644 > > --- a/include/linux/memcontrol.h > > +++ b/include/linux/memcontrol.h > > @@ -625,18 +625,32 @@ static inline bool mem_cgroup_supports_protection(struct mem_cgroup *memcg) > > > > } > > > > -static inline bool mem_cgroup_below_low(struct mem_cgroup *memcg) > > +static inline bool mem_cgroup_ignore_protection(struct mem_cgroup *target, > > + struct mem_cgroup *memcg) > > { > > - if (!mem_cgroup_supports_protection(memcg)) > > How about to merge mem_cgroup_supports_protection() and your new helper into > something like mem_cgroup_possibly_protected()? It seems like they never used > separately and unlikely ever will be used. Sounds good! I am thinking maybe mem_cgroup_no_protection() which is an inlining of !mem_cgroup_supports_protection() || mem_cgorup_ignore_protection(). > Also, I'd swap target and memcg arguments. Sounds good. > > Thank you! > > > PS If it's not too hard, please, consider adding a new kselftest to cover this case. > Thank you! I will try to translate my bash test to something in test_memcontrol, I don't plan to spend a lot of time on it though so I hope it's simple enough..