Thread (45 messages) flat view 45 messages, 3 authors, 6d ago

Re: [PATCH v2 07/26] mm/fbatch: LRU_NEXT_ACTIVATE bit to optimize folio_activate()

From: Hugh Dickins <hughd@google.com>
Date: 2026-09-14 20:19:42
Also in: linux-fsdevel, linux-mm, lkml

On Sat, 12 Sep 2026, Hugh Dickins wrote:
On Thu, 10 Sep 2026, Kiryl Shutsemau wrote:
quoted
On Wed, Sep 09, 2026 at 02:55:41AM -0700, Hugh Dickins wrote:
...
quoted
quoted
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 8420b1276535..8f5efadf9c7c 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -346,6 +346,7 @@ static inline void folio_migrate_refs(struct folio *new, const struct folio *old
 enum {
 	LRU_NEXT_NEVER_TAIL = 0,	/* Used by a tail's compound_head */
 	LRU_NEXT_BATCHED = 1,		/* Not used by any aligned pointer */
+	LRU_NEXT_ACTIVATE,
 	NR_LRU_NEXT_FLAGS
 };
 
@@ -358,6 +359,9 @@ bool lru_add_del_folio(struct folio *folio)
 	if (!(lru_next & BIT(LRU_NEXT_BATCHED)))
 		return false;
 
+	if (lru_next & BIT(LRU_NEXT_ACTIVATE))
+		folio_set_active(folio);
+
 	WRITE_ONCE(folio->lru.next, LIST_POISON1);
 	/* BUG_ON(folio->lru_next & BIT(LRU_NEXT_BATCHED)); */
 
diff --git a/mm/folio.c b/mm/folio.c
index a18d8ef6afd5..0b75c3b69d5a 100644
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -256,15 +256,32 @@ static void lru_activate(struct lruvec *lruvec, struct folio *folio)
 
 void folio_activate(struct folio *folio)
 {
+	unsigned long lru_next;
+
 	if (folio_test_active(folio) || folio_test_unevictable(folio) ||
 	    !folio_test_lru(folio))
 		return;
 
 	/*
-	 * XXX: It is curiously difficult to recreate safely the old
-	 * __lru_cache_activate_folio() optimization (folio_set_active()
-	 * directly if it's on the local lru_add fbatch): revisit later.
+	 * This optimization is intended for the common case of folio
+	 * having been recently added to this CPU's lru_add fbatch.
+	 * But since other CPUs can now take it at any instant (after
+	 * a folio_test_clear_lru()), and we may be migrated to another
+	 * CPU, it is simplest just to extend the optimization to all CPUs.
+	 *
+	 * folio_set_active() would be unsafe without the lruvec lock, and
+	 * a folio_test_clear_lru() here might cause a racing drain of the
+	 * lru_add fbatch to skip its lru_add(): so use try_cmpxchg().
 	 */
+	lru_next = READ_ONCE(folio->lru_next);
+	while (lru_next & BIT(LRU_NEXT_BATCHED)) {
+		if (lru_next & BIT(LRU_NEXT_ACTIVATE))
+			return;
+		if (try_cmpxchg(&folio->lru_next, &lru_next,
+				lru_next | BIT(LRU_NEXT_ACTIVATE)))
+			return;
Hm. What prevents the folio from becoming unevictable under us here?
I don't see anything.

__folio_add_lru() wouldn't like it:

	VM_BUG_ON_FOLIO(folio_test_active(folio) &&
			folio_test_unevictable(folio), folio);

folio_lru_list() has the VM_BUG() too.
You're right, thank you. I thought I had deleted all such VM_BUG_ONs:
and indeed I had, but only in a patch I later decided was too much for
this series (removing PG_unevictable, using !folio_evictable() in some
places, or folio_test_unevictable() testing another POISON in lru_next).

That excuse is not enough for this series! Yes, I must send a fixup,
but not today.
quoted
I am not sure what the right fix is.

Maybe lru_add_del_folio() should only call folio_set_active() on
!folio_test_unevictable() folios?
I was writing the commit message to a 7.1/26 fixup patch,
when I found I just could not describe any possible race here.

(And I was using your first suggestion, above: in the longer term I
prefer what I chose below, but decided it was better not to get into
that now: deleting various VM_BUG_ON_FOLIOs is better argued elsewhere.
There's another of them in folio_migrate_flags().)

folio_activate() has just checked !folio_test_unevictable(), so
it would have to be a race with something which sets the unevictable
flag on this folio at the same time as we find it's LRU_NEXT_BATCHED.

!folio_evictable() might become true at any instant,
but folio_test_unevictable()?

I cannot see what the racer could be: can you? I can see lru_add()
making it unevictable afterwards; and I can see folio migration
(successful or not) carrying unevictable forwards (or setting it
on a freshly allocated folio). But I cannot see any risky race
for folio_activate() or folio_mark_accessed() here.

Hugh
quoted
Or should we allow occasional active+unevictable
Yes, that's what I did, just removed the VM_BUG_ONs: but I'll need
to check again whether that other patch also had to fix any ordering
of checks. Offhand, probably not: once the "Unevictable LRU" became
an oopsing fiction, it was important to check unevictable first:
unevictable must take precedence, and then it really doesn't matter
whether active is set or not.
quoted
so if they are
munlocked, they will go directly to active list?
I didn't think of that, but I don't think that "active", set racily
back when the folio was assigned "unevictable", bears much relation
to whether it ought to be put on active or inactive list when later
made evictable again. We should probably be consistent, and
consistent with existing behaviour, that they go to inactive when
made evictable. (I'm not looking at that other patch at present,
I don't recall where active got cleared in it.)

Hugh
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help