Re: [PATCH v5 6/6] mm/mglru: fix potential generation folio number leak

[Barry Song]

On Wed, Sep 2, 2026 at 5:51 PM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
>
> From: Kairui Song <kasong@xxxxxxxxxxx>
>
> Each generation of MGLRU accounts anon and file folio numbers
> separately, and the page table walker updates each generation's
> counters in batch once the walk is done. The walker promotes a
> folio's generation with a cmpxchg on folio->flags, and
> update_batch_size() then reads the live flags again to pick the
> anon/file column to charge. The walk holds neither the lruvec lock nor
> the folio lock, so the type can flip between the cmpxchg and that
> read: the lazyfree path clears PG_swapbacked, and reclaim sets it back
> on a dirty lazyfree folio. The batched delta pair is then recorded in
> the wrong type column. Nothing reconciles it afterwards, permanently
> skewing lrugen->nr_pages and the reclaim budgets derived from it.
>
> Fix it by capturing the type from the flags snapshot the cmpxchg
> linearized against: folio_update_gen() returns the type of the state
> it transitioned from, and update_batch_size() accounts with that.
>
> A folio's type only changes while it is off the LRU list, inside a
> del/add pair under the lruvec lock, with the gen bits cleared in
> between. The generation and PG_swapbacked sit in the same
> folio->flags word, so the cmpxchg snapshot captures them together.
> Let G be the generation that snapshot captured (old_gen) and G' the
> one it wrote (new_gen); the CAS can land in only three places:
>
>   - before the del: the folio is anon at G; the batch records anon
>     G -> G', and the del later removes the folio from the anon
>     counters;
>   - between del and add: gen == -1, so folio_update_gen() returns -1
>     without touching the flags and no batch is recorded; the del/add
>     pair accounts for the move alone;
>   - after the add: the folio is file at the fresh generation the add
>     charged; the batch records file, that gen -> G', matching that
>     charge.
>
> Unlike the drift of lazy promotions, which sort_folio() repairs under
> the lruvec lock, the phantom deltas from before this fix land in a
> column the folio never occupies again, so nothing ever repairs them.
>
> Fixes: bd74fdaea146 ("mm: multi-gen LRU: support page table walks")
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> ---

Reviewed-by: Barry Song <baohua@xxxxxxxxxx>

[...]

>  /* promote pages accessed through page tables */
> -static int folio_update_gen(struct folio *folio, int new_gen, const vma_flags_t *vma_flags)
> +static int folio_update_gen(struct folio *folio, int new_gen, int *type,
> +                           const vma_flags_t *vma_flags)
>  {
>         unsigned long new_flags, old_flags = READ_ONCE(*folio_flags(folio, 0));
>         int old_gen;
> @@ -3298,6 +3299,7 @@ static int folio_update_gen(struct folio *folio, int new_gen, const vma_flags_t
>                 new_flags |= BIT(PG_workingset);
>         } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
>
> +       *type = folio_flags_is_file_lru(&old_flags);

Though it is a bit ugly that an `is_file` value can be assigned to
a `type`, it is not your fault. We've been doing this throughout
MGLRU. Technically, it should be:

if (folio_flags_is_file_lru(&old_flags))
   *type = LRU_GEN_FILE;
else
  *type = LRU_GEN_ANON;

But that would be too long, so I'm fine with the current code :-)

Best Regards
Barry