Re: [PATCH v3 09/14] mm/mglru: use the common routine for dirty/writeback reactivation

[Kairui Song]

Forwarding sashiko review, which seems not a real issue but just in case:

> commit e6160f371a1c25f6b49b0a6639582d0636d9905b
> Author: Kairui Song <kasong@xxxxxxxxxxx>
> mm/mglru: use the common routine for dirty/writeback reactivation
> This commit drops MGLRU-specific handling for dirty and writeback folios,
> instead allowing the common shrink_folio_list() routine to process and
> reactivate them like the classical LRU.
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 9f4512a4d35fc..2a36cf937061b 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -4628,21 +4627,6 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
> >  		return true;
> >  	}
> >  
> > -	dirty = folio_test_dirty(folio);
> > -	writeback = folio_test_writeback(folio);
> > -	if (type == LRU_GEN_FILE && dirty) {
> > -		sc->nr.file_taken += delta;
> > -		if (!writeback)
> > -			sc->nr.unqueued_dirty += delta;
> > -	}
> > -
> > -	/* waiting for writeback */
> > -	if (writeback || (type == LRU_GEN_FILE && dirty)) {
> > -		gen = folio_inc_gen(lruvec, folio, true);
> > -		list_move(&folio->lru, &lrugen->folios[gen][type][zone]);
> > -		return true;
> > -	}
> Does removing this explicit promotion and instead falling back to 
> shrink_folio_list() cause these folios to retain stale tier bits?
> When dirty and writeback folios are processed by shrink_folio_list(),
> they frequently hit the activate_locked path, which sets PG_active on
> the folio.
> When these folios are returned in a list to evict_folios(), it tries to
> clear stale tier bits by checking if the folio is from the oldest
> generation:
>     if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type])
> Because PG_active is set, lru_gen_folio_seq() calculates a younger
> generation, so this check evaluates to false and the tier bits
> (LRU_REFS_FLAGS) are not cleared.
> Later, the folios are added back to the LRU via move_folios_to_lru() and
> lru_gen_add_folio(). While lru_gen_add_folio() clears PG_active, it
> intentionally does not clear LRU_REFS_FLAGS.
> Could this sequence leave the folios in a younger generation with stale
> PG_referenced and LRU_REFS_MASK bits, artificially inflating their access
> counts?

Actually the new behavior is better I believe, clearing the ref bits for
dirty folios make no sense. We even previously had a commit locally to
disable resetting the refs bits for folio_inc_gen unless it's getting
protected by ref bits (MGLRU's PID).

The access count contributes to the PID protection, refault tracking
and things like PSI, leave these counter untouched should help them to
track the folio hotness info better upon the real reclaim when the
writeback is done.

I better mention this in the commit message indeed.