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 0B22FC7619A for ; Wed, 5 Apr 2023 21:10:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7ECE16B0075; Wed, 5 Apr 2023 17:10:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7759C6B0078; Wed, 5 Apr 2023 17:10:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5EF876B007B; Wed, 5 Apr 2023 17:10:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 4E71E6B0075 for ; Wed, 5 Apr 2023 17:10:09 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 1C843804F6 for ; Wed, 5 Apr 2023 21:10:09 +0000 (UTC) X-FDA: 80648580138.22.E5D79A1 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) by imf04.hostedemail.com (Postfix) with ESMTP id 4A61A40002 for ; Wed, 5 Apr 2023 21:10:06 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=i6chEb9v; spf=pass (imf04.hostedemail.com: domain of yuzhao@google.com designates 209.85.128.51 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680729006; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ghxKNRoR0FZ6RC9U8oDSfHTOCg4Vl7TWo9AvFAhVJL8=; b=69yvIMf/9V0KItEOCrMUy4Bw4k6fPlKTsoa7/BVawxgAgmFIH6JZVQV2htUJZNWJ43j5wm PfzQckl2lvL/rSP7ISu0hpeMzFehR2Tz1tXaf7ks1zvEH5CwO42WILhw6xl6AvAhd4sCC4 oB35trvP7zLJhqbKMRHQHFLQ675UwSA= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=i6chEb9v; spf=pass (imf04.hostedemail.com: domain of yuzhao@google.com designates 209.85.128.51 as permitted sender) smtp.mailfrom=yuzhao@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680729006; a=rsa-sha256; cv=none; b=O6niGKiTLZRYmRbSpW4SoOi97Ers/I8LCbuI4/WgTsrk5xlACkUww+huj2uBNfSFxQReyv ewv7GRgVibj3GOI+Ulza+fD+tqxh8b3vbScvk0LJ1gCSv5h3MhrWaUcHitfwhyCOUJXrSq si3Uww5AcFuRr9ydPtiGn8O7fhBUiKA= Received: by mail-wm1-f51.google.com with SMTP id 5b1f17b1804b1-3ee6c339cceso85055e9.0 for ; Wed, 05 Apr 2023 14:10:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1680729005; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ghxKNRoR0FZ6RC9U8oDSfHTOCg4Vl7TWo9AvFAhVJL8=; b=i6chEb9vGW/gixaqqJDYTVYxj4WC/ZoB6hJEJfG87LKH5mDe8yGKQqXeWDYkcImmmP BUqUgnzI5uUZ98CRt1WBMLCmFoTnaEzW4ndD2uhv4wWQPb8H0c2hSYYr4VxPlDAfmlT2 4M8vDxwmC0WgYubDDkoyWnLN21bNYlU8xHj7my+BI13+AJxRMHcmv9gSImI4Xs2DYwsL C0ZZRPHSgCfPHwl+iQ4OGn3HrF9MYvMA8S0IsX9cL/CrUzbNfzqOxGk4djevBRlS+BZS Bc+o/V1XGTxeWUo1SQgtlPZ8OAKFHiiw2dosuxU88Tn0CDrZU/0M/q+/HDTxMWqa00sT R79A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680729005; h=content-transfer-encoding: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=ghxKNRoR0FZ6RC9U8oDSfHTOCg4Vl7TWo9AvFAhVJL8=; b=i6R/INQq4C5OpQ7DBMQBxtfOEOTjF0YiwzSfQLyQEAsB4ko4lgS+tORU2G1xrqhN4i ELtBfuJ3wL3sfXnKSYyukSnf3rEYCsTtHYn0ZybxRYHKmUMEsbma8XXjpDV1DefY1B3I Mojma4cB5IcLD1eaDdYGDcOsvowN11GfDtO4VJRqr9ZyFyv3BTXrdQ/WLm21+q2f+VBG zlejB+xbXLMiorDOdwEAYKOVyvCUvW2sOhxtwjqCI8BTMC4iwftwGochl6HjdNAZUZcI AzebM4k+mlfm7IyJT5bbjpXVYVPILKr44KkNf3Yuxqh/jxcKQ/cKTpL4QND2VWlvuQkA HoAw== X-Gm-Message-State: AAQBX9eQOg2ISZv/SyCu3uBjXddSmKGNvw6LciSJkSP3tKvaVsOlC3u0 Awy55hDVS2b/F6fptqvvDZ2ppi1SiT0aW7JC+wp3OQ== X-Google-Smtp-Source: AKy350Y8dlY71Bwmyq0R41xrR/A+ov96ltf+DinHLAU+LRjlwWHipLLESWLVC/ufr+l0ytpTF7306GYWNmHIsnWRLKI= X-Received: by 2002:a05:600c:4f8d:b0:3f0:3dd2:8c24 with SMTP id n13-20020a05600c4f8d00b003f03dd28c24mr48308wmq.6.1680729004672; Wed, 05 Apr 2023 14:10:04 -0700 (PDT) MIME-Version: 1.0 References: <20230405200150.GA35884@cmpxchg.org> In-Reply-To: <20230405200150.GA35884@cmpxchg.org> From: Yu Zhao Date: Wed, 5 Apr 2023 15:09:27 -0600 Message-ID: Subject: Re: global_reclaim() and code documentation (was: Re: [PATCH v3 3/3] mm: vmscan: ignore non-LRU-based reclaim in memcg reclaim To: Johannes Weiner Cc: Yosry Ahmed , Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 4A61A40002 X-Stat-Signature: emfqtbwgfagozxn7p5kk8zmcbcqbwbo5 X-Rspam-User: X-HE-Tag: 1680729006-594619 X-HE-Meta: U2FsdGVkX187vhazwETyVdqfe6sSLI8ZS4vxUliElyS6OJrpC+2iqevqrINXRBLBm5LOfYQwnbUyeQmcdwJsPo0TJeVmPdmBHRkjrWNeg58YKfpQVgdJKSn8nbZCVs/C9ElKvE1XBVjL48h65nRRRS21okOliQrjysJUSwYh7cCAiyRLk3Gij1wFZBmUin1I+eQMtVvwfAmun/NpWicmMj+HHByvTWgV6kZTWf5QKA+xBAkqscGwBqNx9vvQQY5sx8+oNCvyztY39HAF4JJwFUNlrV7fjkA0x5s4dJvlFl/fBMxH4WTHlYfv9GNQPwr2OwtIvtDOVaPi7usy0PcHAnXsWEsiB51pOEO7BdhcUpwTI0d/ji5q7v5qxAyOpJpMwWT1AZbkCmM6xlNSJBdcMYjIsHLzPKqI+QVJuOZdsBJgRsQsGQlMvcEV2wg/BxsROHlDfAEitr+MGhs5dDPkObuFi9R7rRg1bmOdTNC+37wExU8aDXsXyzKz/js3Yvlx30A2WurEgXK4FW1PjxAy4X2zoWKUuO0SPpL4RRTEiXN84GMYJsHZEybH0h0154MWo4HXSAIV2vHWVXXy4Yi/ES5B4pX2FY5ApWONw9pODsSe7ctCCFUWnhMAS12H+jD+yZcYNiMq8YqOvtP6T5CG6lshfrCODR9gG4n/pk/qrDu+hN0XgLSTlzv1KNQwjSNDhoskhVRGgqKp0PZ8ubuvXpe9eIvQwZhfIPSVgAghK5TZLwsJawwBh5luQF4WE1bw/2hcoyg/g+eHgmpFbgxfvF0djgtRytZRTcxYRNPAK0EZgbofEEbqdYGLTj0XMajHWyKvryskU24dQtyz/KoaTH73mtD693WGYELEPRF3tAwFTcjt1QttyNwGaHK/kA0tIceyJ5dpmUczSjznVTA4wRuRUzg0K5slgab1rL5l6i7bGUCJ4LAsHJ3i02MjEWvHniXxzt9Yd9zAyJvKAKG J4+wW8zH crZNodbPjewbWfIfKESMuTyQfe1RwYZnm9SkN8mjyfMvS1HJ1jLQrN+38zG5lIXTMllqDe7A9F0Q2Lm+6CEyqU0rGf/hOxhsF9yP9GX/d8eO6DjMi7CeXM4gIeZgyUNz5h7jM+3SX9SaNhs11nn9pMWiBsjQU6EGy5IyLBrz60+WkJxvlQ+t4I00Sr+WZ0n9ouA37P/EGJBuRk7UaQQXaYLa9rmBeUY99tllJhOXnXOjcAhWQ/9Ryo83WtzzVZygL6lNgeWx7/5NWOzaH3LW8GIofNqkSTy9/jI9t6Qa+0ktUbP0/iRtWc/V0fAZ/TdKB3MJWxUbo1b20le70Obdvc4eIgIlSicGi/sAGee+IJUXOdgUyaaZ1XJJZnA== 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 Wed, Apr 5, 2023 at 2:01=E2=80=AFPM Johannes Weiner = wrote: > > On Fri, Mar 31, 2023 at 12:30:11AM -0700, Yosry Ahmed wrote: > > On Fri, Mar 31, 2023 at 12:25=E2=80=AFAM Yu Zhao wr= ote: > > > On Fri, Mar 31, 2023 at 1:08=E2=80=AFAM Yosry Ahmed wrote: > > > > > > ... > > > > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > > > index a3e38851b34ac..bf9d8e175e92a 100644 > > > > --- a/mm/vmscan.c > > > > +++ b/mm/vmscan.c > > > > @@ -533,7 +533,35 @@ EXPORT_SYMBOL(mm_account_reclaimed_pages); > > > > static void flush_reclaim_state(struct scan_control *sc, > > > > struct reclaim_state *rs) > > > > { > > > > - if (rs) { > > > > + /* > > > > + * Currently, reclaim_state->reclaimed includes three types= of pages > > > > + * freed outside of vmscan: > > > > + * (1) Slab pages. > > > > + * (2) Clean file pages from pruned inodes. > > > > + * (3) XFS freed buffer pages. > > > > + * > > > > + * For all of these cases, we have no way of finding out wh= ether these > > > > + * pages were related to the memcg under reclaim. For examp= le, a freed > > > > + * slab page could have had only a single object charged to= the memcg > > > > + * under reclaim. Also, populated inodes are not on shrinke= r LRUs > > > > + * anymore except on highmem systems. > > > > + * > > > > + * Instead of over-reporting the reclaimed pages in a memcg= reclaim, > > > > + * only count such pages in system-wide reclaim. This preve= nts > > > > + * unnecessary retries during memcg charging and false posi= tive from > > > > + * proactive reclaim (memory.reclaim). > > > > > > What happens when writing to the root memory.reclaim? > > > > > > > + * > > > > + * For uncommon cases were the freed pages were actually si= gnificantly > > > > + * charged to the memcg under reclaim, and we end up under-= reporting, it > > > > + * should be fine. The freed pages will be uncharged anyway= , even if > > > > + * they are not reported properly, and we will be able to m= ake forward > > > > + * progress in charging (which is usually in a retry loop). > > > > + * > > > > + * We can go one step further, and report the uncharged obj= cg pages in > > > > + * memcg reclaim, to make reporting more accurate and reduc= e > > > > + * under-reporting, but it's probably not worth the complex= ity for now. > > > > + */ > > > > + if (rs && !cgroup_reclaim(sc)) { > > > > > > To answer the question above, global_reclaim() would be preferred. > > > > Great point, global_reclaim() is fairly recent. I didn't see it > > before. Thanks for pointing it out. I will change it for v4 -- will > > wait for more feedback before respinning. > > I didn't realize it came back, I deleted it a while ago: > > commit b5ead35e7e1d3434ce436dfcb2af32820ce54589 > Author: Johannes Weiner > Date: Sat Nov 30 17:55:40 2019 -0800 > > mm: vmscan: naming fixes: global_reclaim() and sane_reclaim() > > Seven years after introducing the global_reclaim() function, I still = have > to double take when reading a callsite. I don't know how others do i= t, > this is a terrible name. > > Invert the meaning and rename it to cgroup_reclaim(). > > Could you shed some light on why it was brought back? It's not clear > to me from the changelog in a579086c99ed70cc4bfc104348dbe3dd8f2787e6. Sorry about the confusion. I was asking Yosry to post an RFC to clear that = up. > We also now have this: > > static bool cgroup_reclaim(struct scan_control *sc) > { > return sc->target_mem_cgroup; > } > > static bool global_reclaim(struct scan_control *sc) > { > return !sc->target_mem_cgroup || mem_cgroup_is_root(sc->target_me= m_cgroup); > } > > The name suggests it's the same thing twice, with opposite > polarity. But of course they're subtly different, and not documented. > > When do you use which? The problem I saw is that target_mem_cgroup is set when writing to the root memory.reclaim. And for this case, a few places might prefer global_reclaim(), e.g., in shrink_lruvec(), in addition to where it's being used. If this makes sense, we could 1) document it (or rename it) and apply it to those places, or 2) just unset target_mem_cgroup for root and delete global_reclaim(). Option 2 might break ABI but still be acceptable. If we don't want to decide right now, I can rename global_reclaim() to root_reclaim() and move it within #ifdef CONFIG_LRU_GEN and probably come back and revisit later.