Re: [PATCH mm-new] mm/mglru: clarify the generation placement in lru_gen_folio_seq()
From: wangzicheng
Date: Wed Oct 21 2026 - 04:44:43 EST
On 2026-10-20 10:35 +0800, Ridong Chen wrote:
> From: Ridong Chen <chenridong@xxxxxxxxxx>
>
> lru_gen_folio_seq() computes how many generations below max_seq a
> folio should be added to, but stores it in a variable named "gen".
> Elsewhere in MGLRU "gen" means an index into lrugen->folios[], i.e.
> seq % MAX_NR_GENS, so reusing the name for a relative offset is
> confusing.
>
> The comment table above the code also no longer matches it. It only
> covers the PG_active and the final branch, leaving out the reclaiming
> and "can't evict yet" cases, and it lists LRU_REFS_FLAGS as an input
> although lru_gen_folio_seq() never looks at it.
>
> Rename the local variable to "age", redraw the table so that each
> branch maps to the age and seq it produces, and note that the result
> is clamped to min_seq[type].
>
> No functional change.
>
> Assisted-by: LLM
> Signed-off-by: Ridong Chen <chenridong@xxxxxxxxxx>
> ---
> include/linux/mm_inline.h | 48 ++++++++++++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index ab69b9930893..a933d8c14558 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -302,34 +302,50 @@ static inline unsigned long lru_gen_folio_seq(const struct lruvec *lruvec,
> const struct folio *folio,
> bool reclaiming)
> {
> - int gen;
> + int age;
> int type = folio_is_file_lru(folio);
> const struct lru_gen_folio *lrugen = &lruvec->lrugen;
>
> /*
> - * +-----------------------------------+-----------------------------------+
> - * | Accessed through page tables and | Accessed through file descriptors |
> - * | promoted by folio_update_gen() | and protected by folio_inc_gen() |
> - * +-----------------------------------+-----------------------------------+
> - * | PG_active (set while isolated) | |
> - * +-----------------+-----------------+-----------------+-----------------+
> - * | PG_workingset | PG_referenced | PG_workingset | LRU_REFS_FLAGS |
> - * +-----------------------------------+-----------------------------------+
> - * |<---------- MIN_NR_GENS ---------->| |
> - * |<---------------------------- MAX_NR_GENS ---------------------------->|
> + * +-----------------+-----------------+-----------------+-----------------+
> + * age | 1 | 2 | 3 | 4 |
Hi Ridong,
Agreed, I also found this part confusing.
One problem with the new name: in MGLRU a larger seq/gen means a
younger generation, but a larger age means an older one, so every
read needs a flip (max_seq - age + 1). Assigning the seq directly in
each branch might read more naturally.
Also, both tables run past 80 columns (~95 for the art in mglru-fg
4/17, ~96 for the redrawn one here, tab as 8). Would you consider
something vertical and shorter? I had prepared a comment for
mglru-fg, and since this is being discussed here too, here it is
(conditions are per mglru-fg, where the flags became the refs
count):
/*
* The seq is clamped to min_seq[type]:
*
* destination | folio state
* ----------------+--------------------------------------------
* youngest | PG_active, refs >= LRU_REFS_WORKINGSET
* (max_seq) |
*
* 2nd youngest | PG_active, refs < LRU_REFS_WORKINGSET
* (max_seq - 1) | anon folio not yet in swapcache
* | PG_reclaim folio, dirty or under writeback
*
* 2nd oldest | refs >= LRU_REFS_WORKINGSET
* (max_seq - 2) |
*
* oldest | reclaiming
* (min_seq[type]) | otherwise: refs < LRU_REFS_WORKINGSET
*/
With a comment like this, developers can see the generation
placement directly, instead of working it out from the code.
Best,
Zicheng
> + * +-----------------+-----------------+-----------------+-----------------+
> + * seq | max_seq | max_seq-1 | max_seq-2 | max_seq-3 |
> + * +-----------------------------------+-----------------------------------+
> + * |<---------- MIN_NR_GENS ---------->| |
> + * |<---------------------------- MAX_NR_GENS ---------------------------->|
> + * +-----------------------------------+-----------------------------------+
> + * | Accessed through page tables and | Accessed through file descriptors |
> + * | promoted by folio_update_gen() | and protected by folio_inc_gen() |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * PG_active | PG_workingset | !PG_workingset | | |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * reclaiming | | | | always |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * can't | | anon && not in | | |
> + * evict yet | | swapcache, or | | |
> + * | | PG_reclaim && | | |
> + * | | dirty/writeback | | |
> + * +-----------------+-----------------+-----------------+-----------------+
> + * else | | | PG_workingset | !PG_workingset |
> + * | | | or | && |
> + * | | | PG_referenced | !PG_referenced |
> + * +-----------------+-----------------+-----------------+-----------------+
> + *
> + * The resulting seq is clamped to min_seq[type].
> */
> if (folio_test_active(folio))
> - gen = MIN_NR_GENS - folio_test_workingset(folio);
> + age = MIN_NR_GENS - folio_test_workingset(folio);
> else if (reclaiming)
> - gen = MAX_NR_GENS;
> + age = MAX_NR_GENS;
> else if ((!folio_is_file_lru(folio) && !folio_test_swapcache(folio)) ||
> (folio_test_reclaim(folio) &&
> (folio_test_dirty(folio) || folio_test_writeback(folio))))
> - gen = MIN_NR_GENS;
> + age = MIN_NR_GENS;
> else
> - gen = MAX_NR_GENS - (folio_test_workingset(folio) || folio_test_referenced(folio));
> + age = MAX_NR_GENS - (folio_test_workingset(folio) || folio_test_referenced(folio));
>
> - return max(READ_ONCE(lrugen->max_seq) - gen + 1, READ_ONCE(lrugen->min_seq[type]));
> + return max(READ_ONCE(lrugen->max_seq) - age + 1, READ_ONCE(lrugen->min_seq[type]));
> }
>
> static inline bool lru_gen_add_folio(struct lruvec *lruvec, struct folio *folio, bool reclaiming)
> --
> 2.34.1
>
>