All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Barry Song (Xiaomi)" <baohua@kernel.org>
To: axelrasmussen@google.com
Cc: akpm@linux-foundation.org, baohua@kernel.org, kasong@tencent.com,
	lance.yang@linux.dev, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, liulei.rjpt@vivo.com, pfalcato@suse.de,
	qi.zheng@linux.dev, shakeel.butt@linux.dev, surenb@google.com,
	wangzicheng@honor.com, weixugc@google.com, will@kernel.org,
	willy@infradead.org, xueyuan.chen21@gmail.com,
	yuanchu@google.com
Subject: Re: [PATCH] mm/mglru: Use folio_mark_accessed to replace folio_set_active in PF
Date: Tue, 28 Apr 2026 09:35:20 +0800	[thread overview]
Message-ID: <20260428013520.47417-1-baohua@kernel.org> (raw)
In-Reply-To: <CAJHvVcjH+BS306KKOsFcVKkbmkj0aKXtfpuFy+SVYKE4PQmVQw@mail.gmail.com>

On Tue, Apr 28, 2026 at 2:23 AM Axel Rasmussen <axelrasmussen@google.com> wrote:
>
> For what it's worth, I agree with this change in principle.
>
> In production we set fault_around_bytes to 4096. That setting is
> surprisingly load-bearing (i.e. if I change it, even at a small
> experimental scale, I expect workloads to notice and complain). So I
> don't think I have an easy way to test this change under production
> workloads.
>
> Like Andrew said the workload in the commit message doesn't seem
> unreasonable, and the benefit is large.
>
> I guess the workload that would see a downside from this is one that
> heavily uses readahead pages but also generates many "one-time-use"
> pages instead of maintaining a "fixed" working set. Without activating
> the readahead pages, does it lose some of the readahead benefit
> because they are pushed out?
>
> About the Sashiko comments, the tier bits being cleared doesn't seem
> that problematic to me. However, the WORKINGSET_ACTIVATE counter issue
> seems worth fixing.
>

I am considering something more reasonable than simply
"fixing" the counter. Right now, MGLRU unconditionally
treats PF folios as WORKINGSET_ACTIVATE_BASE and neglects
other folios entirely. I am thinking of a better approach
that detects true recency. In the active/inactive case,
this is refault_distance < workingset_size.

In MGLRU, we might detect whether reclamation occurred
within the most recent one or two generations. I am
queuing the following for testing:

diff --git a/mm/workingset.c b/mm/workingset.c
index 07e6836d0502..8b552b3d7e37 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -271,10 +271,11 @@ static void *lru_gen_eviction(struct folio *folio)
  * Fills in @lruvec, @token, @workingset with the values unpacked from shadow.
  */
 static bool lru_gen_test_recent(void *shadow, struct lruvec **lruvec,
-				unsigned long *token, bool *workingset, bool file)
+				unsigned long *token, bool *workingset, bool file,
+				unsigned long *gen_distance)
 {
 	int memcg_id;
-	unsigned long max_seq;
+	unsigned long max_seq, distance;
 	struct mem_cgroup *memcg;
 	struct pglist_data *pgdat;
 
@@ -286,7 +287,10 @@ static bool lru_gen_test_recent(void *shadow, struct lruvec **lruvec,
 	max_seq = READ_ONCE((*lruvec)->lrugen.max_seq);
 	max_seq &= (file ? EVICTION_MASK : EVICTION_MASK_ANON) >> LRU_REFS_WIDTH;
 
-	return abs_diff(max_seq, *token >> LRU_REFS_WIDTH) < MAX_NR_GENS;
+	distance = abs_diff(max_seq, *token >> LRU_REFS_WIDTH);
+	if (gen_distance)
+		*gen_distance = distance;
+	return distance < MAX_NR_GENS;
 }
 
 static void lru_gen_refault(struct folio *folio, void *shadow)
@@ -294,7 +298,7 @@ static void lru_gen_refault(struct folio *folio, void *shadow)
 	bool recent;
 	int hist, tier, refs;
 	bool workingset;
-	unsigned long token;
+	unsigned long token, distance;
 	struct lruvec *lruvec;
 	struct lru_gen_folio *lrugen;
 	int type = folio_is_file_lru(folio);
@@ -302,7 +306,8 @@ static void lru_gen_refault(struct folio *folio, void *shadow)
 
 	rcu_read_lock();
 
-	recent = lru_gen_test_recent(shadow, &lruvec, &token, &workingset, type);
+	recent = lru_gen_test_recent(shadow, &lruvec, &token, &workingset, type,
+				     &distance);
 	if (lruvec != folio_lruvec(folio))
 		goto unlock;
 
@@ -319,9 +324,11 @@ static void lru_gen_refault(struct folio *folio, void *shadow)
 
 	atomic_long_add(delta, &lrugen->refaulted[hist][type][tier]);
 
-	/* see folio_add_lru() where folio_set_active() will be called */
-	if (lru_gen_in_fault())
+	/* If the folio was reclaimed very recently. */
+	if (distance <= MIN_LRU_GENS) {
+		folio_set_active(folio);
 		mod_lruvec_state(lruvec, WORKINGSET_ACTIVATE_BASE + type, delta);
+	}
 
 	if (workingset) {
 		folio_set_workingset(folio);
@@ -442,7 +449,7 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset,
 
 		rcu_read_lock();
 		recent = lru_gen_test_recent(shadow, &eviction_lruvec, &eviction,
-					     workingset, file);
+					     workingset, file, NULL);
 		rcu_read_unlock();
 		return recent;
 	}


  reply	other threads:[~2026-04-28  1:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-18 12:02 [PATCH] mm/mglru: Use folio_mark_accessed to replace folio_set_active in PF Barry Song (Xiaomi)
2026-04-24 11:53 ` Andrew Morton
2026-04-28  5:40   ` Barry Song
2026-04-24 14:10 ` Andrew Morton
2026-04-24 15:19 ` Pedro Falcato
2026-04-26  4:35   ` Barry Song
2026-04-27 14:46     ` Pedro Falcato
2026-04-27 18:22       ` Axel Rasmussen
2026-04-28  1:35         ` Barry Song (Xiaomi) [this message]
2026-04-28  4:24       ` Barry Song
2026-04-24 17:03 ` Shakeel Butt
2026-04-26 21:56   ` Barry Song
2026-04-28 18:54 ` Kairui Song
2026-04-28 22:26   ` Barry Song
2026-04-28 22:50     ` Barry Song
2026-04-29  3:17     ` Kairui Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260428013520.47417-1-baohua@kernel.org \
    --to=baohua@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=kasong@tencent.com \
    --cc=lance.yang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=liulei.rjpt@vivo.com \
    --cc=pfalcato@suse.de \
    --cc=qi.zheng@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=surenb@google.com \
    --cc=wangzicheng@honor.com \
    --cc=weixugc@google.com \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --cc=xueyuan.chen21@gmail.com \
    --cc=yuanchu@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.