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+unevictableYes, 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