Re: [PATCH] mm/mglru: Fix young counter undercount for large folios

On Thu, Aug 13, 2026 at 9:09 AM Baolin Wang
<[email protected]> wrote:
>
>
>
> On 8/13/26 8:53 AM, Barry Song wrote:
> > On Wed, Aug 12, 2026 at 6:17 PM Baolin Wang
> > <[email protected]> wrote:
> >>
> >>
> >>
> >> On 8/12/26 2:59 PM, Hui Zhu wrote:
> >>> From: Hui Zhu <[email protected]>
> >>>
> >>> In lru_gen_look_around(), the young counter tracks the number of young
> >>> PTEs.  The original folio's contribution is represented by the initial
> >>> value of young: test_and_clear_young_ptes_notify() is called on it at
> >>> function entry, and the function returns early if it is not young.  In
> >>> the subsequent loop, the original folio is skipped (its accessed bits
> >>> were already cleared), so it is not double-counted.
> >>>
> >>> However, young is initialized to 1 regardless of the folio size.  When
> >>> the original folio is a large folio with nr PTEs, its young count is
> >>> underestimated by nr - 1.  This inconsistency can cause
> >>> suitable_to_scan() to return false, preventing the PMD from being added
> >>> to the bloom filter and reducing aging accuracy for mTHP workloads.
> >>>
> >>> Initialize young to nr so the original folio is accounted the same way
> >>> as other young folios in the loop (young += nr).
> >>>
> >>> Signed-off-by: Hui Zhu <[email protected]>
> >>> ---
> >>
> >> Good catch. Please also add the Fixes tag:
> >>
> >> Fixes: 56e5b60b2114 ("mm: support batched checking of the young flag for
> >> MGLRU")
> >>
> >> With that,
> >> Reviewed-by: Baolin Wang <[email protected]>
> >
> > Hi Baolin, Hui,
> >
> > I am not convinced this is the correct patch. test_and_clear_young_ptes_notify()
> > only indicates that there is at least one young PTE among the nr PTEs;
> > it does not mean that all of the PTEs are young.
> >
> > Am I missing something?
>
> You are right. But I explained why this is done in my original commit
> 56e5b60b2114:
>
> "
> Note that we also update the 'young' counter and
> 'mm_stats[MM_LEAF_YOUNG]' counter with the batched count in the
> lru_gen_look_around() and walk_pte_range().  However, the batched
> operations may inflate these two counters, because in a large folio not
> all PTEs may have been accessed. (Additionally, tracking how many PTEs
> have been accessed within a large folio is not very meaningful, since
> the mm core actually tracks access/dirty on a per-folio basis, not per
> page).  The impact analysis is as follows:
>
> 1. The 'mm_stats[MM_LEAF_YOUNG]' counter has no functional impact and is
> mainly for debugging.
>
> 2. The 'young' counter is used to decide whether to place the current
> PMD entry into the bloom filters by suitable_to_scan() (so that next
> time we can check whether it has been accessed again), which may set the
> hash bit in the bloom filters for a PMD entry that hasn't seen much
> access.  However, bloom filters inherently allow some error, so this
> effect appears negligible.
> "
>
> Based on this, I think changing it to 'nr' is reasonable. For an
> accessed large folio, it's better to have the bloom filter rescan the
> PMD and keep it in memory instead of reclaiming it incorrectly.
>

I am not sure if this is the best policy, but we don't seem to have
a practical way to get the exact number of accessed PTEs, so this may
be acceptable. However, could we at least update the changelog to
clarify that this is intentional?

" However, young is initialized to 1 regardless of the folio size.  When
 the original folio is a large folio with nr PTEs, its young count is
 underestimated by nr - 1.  This inconsistency can cause
 suitable_to_scan() to return false, preventing the PMD from being added"

Its young count is not underestimated; we are intentionally
overestimating it. Also, nr does not necessarily equal
folio_nr_pages(), does it?