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

[Kiryl Shutsemau <kas@kernel.org>]

On Mon, Sep 14, 2026 at 01:19:37PM -0700, Hugh Dickins wrote:
> 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.

I was thinking of race with mlock() plus a failed compaction (assuming
compact_unevictable_allowed is 1):

	CPU0						CPU1
folio_activate()
  folio_test_unevictable() == false
  folio_test_lru() == true
						__mlock_folio()
						  folio_set_unevictable()
						  <moves to LRU_UNEVICTABLE>
						...
						compact_zone()
						  isolate_migratepages()
						    isolate_migratepages_block()
						      folio_test_clear_lru()
						      lruvec_del_folio()
						  <migration fails>
						  putback_movable_pages()
						    folio_putback_lru()
						      __folio_add_lru() // !active, unevictable
						        lru_next |= BIT(LRU_NEXT_BATCHED);
						        folio_set_lru()
  lru_next = READ_ONCE(lru_next)
  // lru_next has LRU_NEXT_BATCHED
  try_cmpxchg(lru_next | LRU_NEXT_ACTIVATE) == true

-- 
  Kiryl Shutsemau / Kirill A. Shutemov