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

[Hugh Dickins <hughd@google.com>]

On Thu, 10 Sep 2026, Kiryl Shutsemau wrote:
> On Wed, Sep 09, 2026 at 02:55:41AM -0700, Hugh Dickins wrote:
> > Implement an equivalent to the old __lru_cache_activate_folio()
> > optimization, to activate a folio recently put in the lru_add fbatch,
> > without having to put it through the lru_activate fbatch too. Neither
> > lruvec lock nor lru bit can guard this safely and efficiently, so resort
> > to try_cmpxchg() on a further, LRU_NEXT_ACTIVATE bit in folio->lru_next.
> > 
> > Signed-off-by: Hugh Dickins <hughd@google.com>
> > ---
> >  include/linux/mm_inline.h |  4 ++++
> >  mm/folio.c                | 23 ++++++++++++++++++++---
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> > 
> > 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.

> 
> 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?
> 
> 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.

> 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