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

[Kiryl Shutsemau <kas@kernel.org>]

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.

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 so if they are
munlocked, they will go directly to active list?

> +	}
> +
>  	folio_batch_add_and_move(folio, lru_activate);
>  }
>  
> -- 
> 2.51.0
> 

-- 
  Kiryl Shutsemau / Kirill A. Shutemov