Re: [PATCH v4 13/17] mm/mglru: folio LRU refs based active/inactive number accounting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: MGLRU tier bits (LRU_REFS_FLAGS) not cleared when updating LRU_GEN_MASK
--

commit 97ee7d2a683879e87f8e211218beb97d5cbbba1b
Author: Kairui Song <kasong@tencent.com>

mm/mglru: folio LRU refs based active/inactive number accounting

This commit updates the active/inactive LRU size tracking to use
per-folio referenced counts instead of the generation window. Folios
are now accounted as active once their references reach LRU_REFS_ACTIVATED.

> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index 6c9510002754b..41c2e9150c709 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
[ ... ]
> @@ -374,7 +373,7 @@ static inline bool lru_gen_add_folio(struct lruvec *lruvec, struct folio *folio,
>  	struct lru_gen_folio *lrugen = &lruvec->lrugen;
>  
>  	BUILD_BUG_ON(BIT(LRU_GEN_WIDTH - 1) != MAX_NR_GENS);
> -	VM_WARN_ON_ONCE_FOLIO(gen != -1, folio);
> +	VM_WARN_ON_ONCE_FOLIO(folio_lru_gen(folio) != -1, folio);
>  
>  	if (folio_test_unevictable(folio) || !lrugen->enabled)
>  		return false;
> @@ -383,10 +382,12 @@ static inline bool lru_gen_add_folio(struct lruvec *lruvec, struct folio *folio,
>  	gen = lru_gen_from_seq(seq);
>  	flags = (gen + 1UL) << LRU_GEN_PGOFF;
>  	/* see the comment on MIN_NR_GENS about PG_active */
> -	set_mask_bits(folio_flags(folio, 0), LRU_GEN_MASK | BIT(PG_active), flags);
> +	flags = set_mask_bits(folio_flags(folio, 0), LRU_GEN_MASK | BIT(PG_active), flags);

[Severity: Medium]
Should LRU_REFS_FLAGS be included in the mask cleared by set_mask_bits() here?

When a folio is added to a generation and LRU_GEN_MASK is modified, leaving
stale tier bits (LRU_REFS_FLAGS) intact can allow them to persist. This
could inadvertently inflate access counts and distort future eviction tracking.

> +	/* use the refs from the atomic snapshot to avoid raced update */
> +	refs = lru_get_refs_flags(flags);
>  
>  	lru_gen_update_size(lruvec, folio, -1, gen);
[ ... ]
> @@ -402,24 +403,31 @@ static inline bool lru_gen_add_folio(struct lruvec *lruvec, struct folio *folio,
>  static inline bool lru_gen_del_folio(struct lruvec *lruvec, struct folio *folio, bool reclaiming)
>  {
>  	unsigned long flags;
> -	int gen = folio_lru_gen(folio);
> +	int gen, refs;
>  	int zone = folio_zonenum(folio);
>  	int delta = folio_nr_pages(folio);
>  	enum lru_list lru = folio_is_file_lru(folio) * LRU_INACTIVE_FILE;
> +	unsigned long max_seq = READ_ONCE(lruvec->lrugen.max_seq);
>  
> +	flags = set_mask_bits(folio_flags(folio, 0), LRU_GEN_MASK, 0);

[Severity: Medium]
Could this mask also be missing the LRU_REFS_FLAGS bit clear?

Similar to the addition path above, when isolating a folio and clearing
LRU_GEN_MASK, the tier tracking bits might also need to be explicitly
cleared so they are not improperly carried over.

> +	gen = lru_get_gen_flags(flags);
> +	refs = lru_get_refs_flags(flags);
>  	if (gen < 0)
>  		return false;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-mglru-fg-v4-0-2106750c0bfa@tencent.com?part=13