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 B91A6C433F5 for ; Thu, 3 Mar 2022 15:29:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4FF3A8D0002; Thu, 3 Mar 2022 10:29:48 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4D4818D0001; Thu, 3 Mar 2022 10:29:48 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 39C158D0002; Thu, 3 Mar 2022 10:29:48 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.27]) by kanga.kvack.org (Postfix) with ESMTP id 2D2538D0001 for ; Thu, 3 Mar 2022 10:29:48 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E7F3725608 for ; Thu, 3 Mar 2022 15:29:47 +0000 (UTC) X-FDA: 79203460014.13.7AC5956 Received: from mail-qk1-f174.google.com (mail-qk1-f174.google.com [209.85.222.174]) by imf13.hostedemail.com (Postfix) with ESMTP id 3636E20015 for ; Thu, 3 Mar 2022 15:29:47 +0000 (UTC) Received: by mail-qk1-f174.google.com with SMTP id z66so4115840qke.10 for ; Thu, 03 Mar 2022 07:29:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=T8thn2mB/ppQpWbyQ7im9TnM2/JxxytI355jY/Bhilk=; b=vb2g9+UTXGE9JVUAdFvLQFaK40Fzo/KR/Wyc1iAd70hZcpP4J7qYBRo9ziJ6pTWNpo GHqdNARWvNGxXTRVc4scAxB45F+vWsKAXB5VpQz2wIxiXGbLsPP2J9yuiz2Gu7Kcv9CM w+LLICGUrWrOW0ZXRHk6I1XCBwEUA3lMhwXPc5vcqyyk5ZL+lwLZ1eV83Qk7lFqdCDEM VqPvdRlUkMz1aaBHWmpuylcdNqwmn1oeaDB9IFS2Y8EmNUeT7E8xUZGHoni8kLsxj3Ul 3xL1pSY+UOYTKRE5FcOo178orvQkcxSIQ6yB8Qo8ld4J+y66PK8LUE8lMnlRu7lji/uY JyXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=T8thn2mB/ppQpWbyQ7im9TnM2/JxxytI355jY/Bhilk=; b=kmk8VuWz9v5G0jc8ulYVV1BaZDeqyVDRACGRGjsNBHPnK0WhLCgQs6lNw3PHsnKDbk PnJOMjgIcSY43zicG2mYacHt6Kg6e4CLdP/wH5Y530uf+AeBcElEsj2fbFT8Ebc2Dhe/ mYsaRn1xD8t/oz155Vuuqg+JnHmalxl23RTE3I4QqdVKLUqKAGlSaUpPt0erOsDSIYyG v/LsjqtobkutcEZESsTzA5bAQ+nAEOXdVuRVKAd778TaK6M2a24ew1TxoPRT5v+rBgXy +Oyibi6Fw3cJNtYozntOS5tA2FKqb3mP9u8TMQaqeWh/mhe1mttDj5ob5dyRULpnr0FA eErQ== X-Gm-Message-State: AOAM5307WOoEXrmZB8LtPKy16eIQD4bqyFtQYUpdctSsHkFBiRdm8fsy E47EBsSFBqDkQs1jRcERwAtgTw== X-Google-Smtp-Source: ABdhPJwvEGkm3dghfQ28xfPR4A725Ug3F6RrYEBmX/tBy75H3GfZgPVNvLYLSVgRGJT6mAqdfRNOew== X-Received: by 2002:ae9:ed55:0:b0:60d:d41c:9e9c with SMTP id c82-20020ae9ed55000000b0060dd41c9e9cmr18783481qkg.276.1646321386184; Thu, 03 Mar 2022 07:29:46 -0800 (PST) Received: from localhost (cpe-98-15-154-102.hvc.res.rr.com. [98.15.154.102]) by smtp.gmail.com with ESMTPSA id i3-20020a05620a074300b006630cbe7ec0sm1291038qki.90.2022.03.03.07.29.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 03 Mar 2022 07:29:45 -0800 (PST) Date: Thu, 3 Mar 2022 10:29:44 -0500 From: Johannes Weiner To: Yu Zhao Cc: Andrew Morton , Mel Gorman , Michal Hocko , Andi Kleen , Aneesh Kumar , Barry Song <21cnbao@gmail.com>, Catalin Marinas , Dave Hansen , Hillf Danton , Jens Axboe , Jesse Barnes , Jonathan Corbet , Linus Torvalds , Matthew Wilcox , Michael Larabel , Mike Rapoport , Rik van Riel , Vlastimil Babka , Will Deacon , Ying Huang , linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, page-reclaim@google.com, x86@kernel.org, Brian Geffon , Jan Alexander Steffens , Oleksandr Natalenko , Steven Barrett , Suleiman Souhlal , Daniel Byrne , Donald Carr , Holger =?iso-8859-1?Q?Hoffst=E4tte?= , Konstantin Kharlamov , Shuang Zhai , Sofia Trinh Subject: Re: [PATCH v7 04/12] mm: multigenerational LRU: groundwork Message-ID: References: <20220208081902.3550911-1-yuzhao@google.com> <20220208081902.3550911-5-yuzhao@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 3636E20015 X-Stat-Signature: w8763616j8rs9bfa8sjy639jnrtq7sbf X-Rspam-User: Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=vb2g9+UT; spf=pass (imf13.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.222.174 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org X-Rspamd-Server: rspam07 X-HE-Tag: 1646321387-106730 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: Hi Yu, On Mon, Feb 21, 2022 at 01:14:24AM -0700, Yu Zhao wrote: > On Tue, Feb 15, 2022 at 04:53:56PM -0500, Johannes Weiner wrote: > > On Tue, Feb 15, 2022 at 02:43:05AM -0700, Yu Zhao wrote: > > > On Thu, Feb 10, 2022 at 03:41:57PM -0500, Johannes Weiner wrote: > > > > > +static inline bool lru_gen_is_active(struct lruvec *lruvec, int gen) > > > > > +{ > > > > > + unsigned long max_seq = lruvec->lrugen.max_seq; > > > > > + > > > > > + VM_BUG_ON(gen >= MAX_NR_GENS); > > > > > + > > > > > + /* see the comment on MIN_NR_GENS */ > > > > > + return gen == lru_gen_from_seq(max_seq) || gen == lru_gen_from_seq(max_seq - 1); > > > > > +} > > > > > > > > I'm still reading the series, so correct me if I'm wrong: the "active" > > > > set is split into two generations for the sole purpose of the > > > > second-chance policy for fresh faults, right? > > > > > > To be precise, the active/inactive notion on top of generations is > > > just for ABI compatibility, e.g., the counters in /proc/vmstat. > > > Otherwise, this function wouldn't be needed. > > > > Ah! would you mind adding this as a comment to the function? > > Will do. > > > But AFAICS there is the lru_gen_del_folio() callsite that maps it to > > the PG_active flag - which in turn gets used by add_folio() to place > > the thing back on the max_seq generation. So I suppose there is a > > secondary purpose of the function for remembering the page's rough age > > for non-reclaim isolation.> > > Yes, e.g., migration. Ok, thanks for clarifying. That should also be in the comment. On scan resistance: > > The concern isn't the scan overhead, but jankiness from the workingset > > being flooded out by streaming IO. > > Yes, MGLRU uses a different approach to solve this problem, and for > its approach, the scan overhead is the concern. > > MGLRU detects (defines) the working set by scanning the entire memory > for each generation, and it counters the flooding by accelerating the > creation of generations. IOW, all mapped pages have an equal chance to > get scanned, no matter which generation they are in. This is a design > difference compared with the active/inactive LRU, which tries to scans > the active/inactive lists less/more frequently. > > > The concrete usecase at the time was a torrent client hashing a > > downloaded file and thereby kicking out the desktop environment, which > > caused jankiness. The hashing didn't benefit from caching - the file > > wouldn't have fit into RAM anyway - so this was pointless to boot. > > > > Essentially, the tradeoff is this: > > > > 1) If you treat new pages as hot, you accelerate workingset > > transitions, but on the flipside you risk unnecessary refaults in > > running applications when those new pages are one-off. > > > > 2) If you take new pages with a grain of salt, you protect existing > > applications better from one-off floods, but risk refaults in NEW > > application while they're trying to start up. > > Agreed. > > > There are two arguments for why 2) is preferable: > > > > 1) Users are tolerant of cache misses when applications first launch, > > much less so after they've been running for hours. > > Our CUJs (Critical User Journeys) respectfully disagree :) > > They are built on the observation that once users have moved onto > another tab/app, they are more likely to stay with the new tab/app > rather than go back to the old ones. Speaking for myself, this is > generally the case. That's in line with what I said. Where is the disagreement? > > 2) Workingset transitions (and associated jankiness) are bounded by > > the amount of RAM you need to repopulate. But streaming IO is > > bounded by storage, and datasets are routinely several times the > > amount of RAM. Uncacheable sets in excess of RAM can produce an > > infinite stream of "new" references; not protecting the workingset > > from that means longer or even sustained jankiness. > > I'd argue the opposite -- we shouldn't risk refaulting fresh hot pages > just to accommodate this concrete yet minor use case, especially > considering torrent has been given the means (MADV_SEQUENTIAL) to help > itself. > > I appreciate all your points here. The bottom line is we agree this is > a trade off. For what disagree about, we could be both right -- it > comes down to what workloads we care about *more*. It's a straight-forward question: How does MGLRU avoid cache pollution from scans? Your answer above seems to be "it just does". Your answer here seems to be "it doesn't, but it doesn't matter". Forgive me if I'm misreading what you're saying. But it's not a minor concern. Read the motivation behind any modern cache algorithm - ARC, LIRS, Clock-Pro, LRU-K, 2Q - and scan resistance is the reason for why they all exist in the first place. "The LRU-K algorithm surpasses conventional buffering algorithms in discriminating between frequently and infrequently referenced pages." - The LRU-K page replacement algorithm for database disk buffering, O'Neil et al, 1993 "Although LRU replacement policy has been commonly used in the buffer cache management, it is well known for its inability to cope with access patterns with weak locality." - LIRS: an efficient low inter-reference recency set replacement policy to improve buffer cache performance, Jiang, Zhang, 2002 "The self-tuning, low-overhead, scan-resistant adaptive replacement cache algorithm outperforms the least-recently-used algorithm by dynamically responding to changing access patterns and continually balancing between workload recency and frequency features." - Outperforming LRU with an adaptive replacement cache algorithm, Megiddo, Modha, 2004 "Over the last three decades, the inability of LRU as well as CLOCK to handle weak locality accesses has become increasingly serious, and an effective fix becomes increasingly desirable. - CLOCK-Pro: An Effective Improvement of the CLOCK Replacement, Jiang et al, 2005 We can't rely on MADV_SEQUENTIAL alone. Not all accesses know in advance that they'll be one-off; it can be a group of uncoordinated tasks causing the pattern etc. This is a pretty fundamental issue. It would be good to get a more satisfying answer on this. > > > > You can drop the memcg parameter and use lruvec_memcg(). > > > > > > lruvec_memcg() isn't available yet when pgdat_init_internals() calls > > > this function because mem_cgroup_disabled() is initialized afterward. > > > > Good catch. That'll container_of() into garbage. However, we have to > > assume that somebody's going to try that simplification again, so we > > should set up the code now to prevent issues. > > > > cgroup_disable parsing is self-contained, so we can pull it ahead in > > the init sequence. How about this? > > > > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c > > index 9d05c3ca2d5e..b544d768edc8 100644 > > --- a/kernel/cgroup/cgroup.c > > +++ b/kernel/cgroup/cgroup.c > > @@ -6464,9 +6464,9 @@ static int __init cgroup_disable(char *str) > > break; > > } > > } > > - return 1; > > + return 0; > > } > > -__setup("cgroup_disable=", cgroup_disable); > > +early_param("cgroup_disable", cgroup_disable); > > I think early_param() is still after pgdat_init_internals(), no? It's called twice for some reason, but AFAICS the first one is always called before pgdat_init_internals(): start_kernel() setup_arch() parse_early_param() x86_init.paging.pagetable_init(); paging_init() zone_sizes_init() free_area_init() free_area_init_node() free_area_init_core() pgdat_init_internals() parse_early_param() It's the same/similar for arm, sparc and mips.