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 0E261C4167B for ; Mon, 4 Dec 2023 19:52:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 828FC6B02E7; Mon, 4 Dec 2023 14:52:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7D6F66B02F1; Mon, 4 Dec 2023 14:52:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 69F876B02FA; Mon, 4 Dec 2023 14:52:09 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5B9C96B02E7 for ; Mon, 4 Dec 2023 14:52:09 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 25A69120357 for ; Mon, 4 Dec 2023 19:52:09 +0000 (UTC) X-FDA: 81530181978.29.939B1C2 Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) by imf24.hostedemail.com (Postfix) with ESMTP id 64C70180020 for ; Mon, 4 Dec 2023 19:52:07 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=3U270Lmq; spf=pass (imf24.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.47 as permitted sender) smtp.mailfrom=yosryahmed@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=1701719527; 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=eYidRuFuyjgtGaepFNp4s5KekmZ8R0Vv2VOTsdlcvvk=; b=d4qxm0ivy9jbmhzaVme0Z7SOnJu/B5phh0q+1cvmkgQvAX0LUF+TazOBFf8ujrIQopq4KQ Vsgi0qJHj67CwKNeefZ7WIpETY3oW+FMysvw4ZSbX9Sft3U6Swv+Ss6rOY42+/zvT1JX8C IeGnP+KMVOTZsHL7wWihfPF/dkxkI6I= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=3U270Lmq; spf=pass (imf24.hostedemail.com: domain of yosryahmed@google.com designates 209.85.208.47 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701719527; a=rsa-sha256; cv=none; b=dhqzGHkPdJjIJSPEKQAABC/EOtimFomyMtPYjrxpx/yQdTRs4fMv4wm7eAFLPb04UnMDcP sSOXuaWS0XolXOAk8iMMWaiqfDapbWJpZv/s97QyvSLdVgm8A6szHaS75EAuu9k0T1nDzL uJ/OgfYBZjZgs+zBphZQTHxZp2HZfZI= Received: by mail-ed1-f47.google.com with SMTP id 4fb4d7f45d1cf-54bfa9b3ffaso6248921a12.1 for ; Mon, 04 Dec 2023 11:52:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1701719526; x=1702324326; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=eYidRuFuyjgtGaepFNp4s5KekmZ8R0Vv2VOTsdlcvvk=; b=3U270Lmqj8zaBmUdW5Aqa1OZiYHY2ENP0HLZzzxO4YviK1ppyQvbUss5y1oJCM7zkN boBvwvKrC/ZBa6YuPVGdxFcOnqjRGwuecre7cx23bC19JC07rp4/4tI6y/dUWAsFrk+8 hGrs3NyGfUR2jkrLima3ORTzM/jWPdCXIZlEnwvElrgp9MjIaLyBgD779w2SnZuDRd19 ugLKX7dg8yINojnFzM/D2rS2YwKGH8HFoahd/rJtqS4xeJh6GKPLUNOSO4oxCebvFA6W uE0Ft3UJrrrqS79Db4zm6QMxe4qmpaVKnhUEz3mzLV6pUDKD8p7VBCOSFM6Go7tde5s6 +Kyg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701719526; x=1702324326; 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=eYidRuFuyjgtGaepFNp4s5KekmZ8R0Vv2VOTsdlcvvk=; b=HU8hrB7v2aJBuA8L1hrWe6m3WPLJA/elHJxUX+3rm9ObV3VNLgUsdhhbf3aznx5wwk sX+ehNSjBKwKMTbZQH2IqzjI/Dfw+dK68t3Oz+AT2hC+X4Zv80MXxTsPCGGEY+d8e/LA olqEr1z5jUqLu7CfYPKQiCpDyjfCxtP8alUdx/ZXr2+e7eW/CZpTXlyx10COJQXwP/r/ 9YvsINii2/PpMGUO0oNot7yOaKg/g1H9B6wAy8e9iI2HhAjCCAxIaWchIHYG90DUp8b2 3TuAdSgryWpx7i6IKrW9QAUe6SrNZqgGjIbnFYA7xKPvVjqhZaeKqqxN7R71PwpNZ11t M4LQ== X-Gm-Message-State: AOJu0YwuAB1le+VvJYIPh41S/+et/lM85Ozjz8c2JvSBLzQf7b2mHUdp aOV3MmQM4r+Bq0oEkXE39uFAVIP/Rt+eP3Saq/vAug== X-Google-Smtp-Source: AGHT+IFlwZwLSTjml6GJX9pmadhYbMF7fdFHsa20v/3WIXVEPDjbDlUP+SUIT8iByELIZI3zCmeFa5ieAkvSDn1t5m8= X-Received: by 2002:a17:906:2207:b0:a16:37ce:f81a with SMTP id s7-20020a170906220700b00a1637cef81amr3789708ejs.8.1701719525571; Mon, 04 Dec 2023 11:52:05 -0800 (PST) MIME-Version: 1.0 References: <20231129032154.3710765-1-yosryahmed@google.com> <20231129032154.3710765-6-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Mon, 4 Dec 2023 11:51:29 -0800 Message-ID: Subject: Re: [mm-unstable v4 5/5] mm: memcg: restore subtree stats flushing To: Bagas Sanjaya Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Ivan Babrou , Tejun Heo , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Waiman Long , kernel-team@cloudflare.com, Wei Xu , Greg Thelen , Domenico Cerasuolo , Attreyee M , Linux Memory Management List , Linux CGroups , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 64C70180020 X-Rspam-User: X-Stat-Signature: 1r3r98sft1d5twchsh4g1daedjn6q8z4 X-Rspamd-Server: rspam01 X-HE-Tag: 1701719527-492232 X-HE-Meta: U2FsdGVkX18Ae9F5FefYZJbhO9160qee8IPcGoYklzZPtTq53zuRkICRnBZt5QNU6QpoCt1iJ7XZxZ4FMpvNzqnyBXcGGTOZwOVvsu4HllSFCi/nWQOthzz3TUplIo8iCUSCo5Y3ExIKKsQhnOV8J69QOG+PFPkU7ipklrrzFZjCU4wSbhiPNmfGRVEDNMAbasBVnHlAbMqdw2C6d1hCT20PZDOGnJiKWosgXULS+XAxBhCPqoctbh0AIrpLrWWJ3mcMH/PL3v8ILF160cMp85mLxNHjDtCYCdXUKYuPmMBRm7Fmkq3KjwGDFqA3ZcvPtEZnFPlKyc0iVuwwvMfLW1xIvm/6YRr+AYTacHToUg/mAfL1DZrauOzFUSGzIh5EOr275Tu6DOuErTjnCs/tnmPCgIVxwmMy6wgb0tWabbv3C1UNvVhOFnbeCzVvswuNhJWeOU9UIjL7ssAQQy4AGKX0mf9DwHu27PLRMgRpxCecB3mgACZfSaFh6yAGtaKClQoMHP+Jfu5wxK6DYplVL9QchGu0NHJo1LGvYYtk/vBPbdxaEbYojhhZwDfgYFTpOqRl/LR+yI2gwkSctvQ/tlhcJ9PPB+3qkxugcOi2mmaufeGqHlRBgMlXXQtF/aQY8Muu4ATlb+Ky5ZgUd8Rjsn6gAXYWq54r98lvVAd9A5117q9n3MP7MmQPOVD8wqSUjXPJCGunZ7Cu1vSJ3lqMv8dX3Cyf0F7Nsih+wEpc+yzOYHk/ZpaaF2gB797+M6fHvYromo6JO2EQ6Wple39kWuNqQCd9aHGU61nlH7ntof23GUt48JvKv5GGsRwXVySu1rYKJB5aEU1wNe0V06YxbGomkK+DB5DuksmjebYsmSX6gH1DyD481DQeAfihCQZeN/Zzuv1dhMI5EC/WAjHx1GeBGCxOIG6VlUIi/++OloETJ+bRWLYxO5ovYoQ6GaHlgapY8q1HWTT1jdNmE/U /uflZ/3q yhBamTZYKcU325zSomJQYfd2Kbrz76YCx2YwtYwuXv2JSayF5awovhYZR9ZisXsWkxnG2ZqSawszL/wDJ7cYimjS/XXoxzo9T7FR9Kj1OYTyFTpodFY6Wwe2giSndeJGF3OcJad9tNIqY6dID/4Cw1nkfnZpFL6oH59a4H1F+wjPjmXYdlcsKJ1EJhj+uYiRvUr8BbnR+3+1POZdJhA9YAkg0zWme8GOe80RO 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: List-Subscribe: List-Unsubscribe: [..] > > diff --git a/mm/workingset.c b/mm/workingset.c > > index dce41577a49d2..7d3dacab8451a 100644 > > --- a/mm/workingset.c > > +++ b/mm/workingset.c > > @@ -464,8 +464,12 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset) > > > > rcu_read_unlock(); > > > > - /* Flush stats (and potentially sleep) outside the RCU read section */ > > - mem_cgroup_flush_stats_ratelimited(); > > + /* > > + * Flush stats (and potentially sleep) outside the RCU read section. > > + * XXX: With per-memcg flushing and thresholding, is ratelimiting > > + * still needed here? > > + */ > > + mem_cgroup_flush_stats_ratelimited(eviction_memcg); > > What if flushing is not rate-limited (e.g. above line is commented)? > Hmm I think I might be misunderstanding the question. The call to mem_cgroup_flush_stats_ratelimited() does not ratelimit other flushers, it is rather a flush call that is itself ratelimited. IOW, it may or may not flush based on when was the last time someone else flushed. This was introduced because flushing in the fault path was expensive in some cases, so we wanted to avoid flushing if someone else recently did a flush, as we don't expect a lot of pending changes in this case. However, that was when flushing was always on the root level. Now that we are flushing on the memcg level, it may no longer be needed as: - The flush is more scoped, there should be less work to do. - There is a per-memcg threshold now such that we only flush when there are pending updates in this memcg. This is why I added a comment that the ratelimited flush here may no longer be needed. I didn't want to investigate this as part of this series, especially that I do not have a reproducer for the fault latency introduced by the flush before ratelimiting. Hence, I am leaving the comment such that people know that this ratelimiting may no longer be needed with this patch. > > > > eviction_lruvec = mem_cgroup_lruvec(eviction_memcg, pgdat); > > refault = atomic_long_read(&eviction_lruvec->nonresident_age); > > @@ -676,7 +680,7 @@ static unsigned long count_shadow_nodes(struct shrinker *shrinker, > > struct lruvec *lruvec; > > int i; > > > > - mem_cgroup_flush_stats(); > > + mem_cgroup_flush_stats(sc->memcg); > > lruvec = mem_cgroup_lruvec(sc->memcg, NODE_DATA(sc->nid)); > > for (pages = 0, i = 0; i < NR_LRU_LISTS; i++) > > pages += lruvec_page_state_local(lruvec, > > Confused... Which part is confusing? The call to mem_cgroup_flush_stats() now receives a memcg argument as flushing is scoped to that memcg only to avoid doing unnecessary work to flush other memcgs with global flushing.