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

[Hugh Dickins <hughd@google.com>]

On Sat, 12 Sep 2026, Hugh Dickins wrote:
> On Thu, 10 Sep 2026, Kiryl Shutsemau wrote:
> > On Wed, Sep 09, 2026 at 02:55:41AM -0700, Hugh Dickins wrote:
...
> > > 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?

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

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