Re: [PATCH mm-unstable v3 2/2] mm/vmscan: apply too_many_isolated() throttling to MGLRU eviction

[Baolin Wang]

On 8/19/26 6:06 AM, Barry Song wrote:
> On Tue, Aug 18, 2026 at 8:49 PM Hui Zhu <hui.zhu@xxxxxxxxx> wrote:
> > From: Hui Zhu <zhuhui@xxxxxxxxxx>
> >
> > The legacy path throttles direct reclaim in shrink_inactive_list()
> > when too many isolated folios pile up, but MGLRU's evict_folios()
> > isolates folios without this check, which can lead to unnecessary
> > swapping, thrashing and OOM.
> >
> > With the NR_ISOLATED counters now updated in evict_folios(), extract
> > the throttling loop from shrink_inactive_list() into
> > throttle_is_throttled() and reuse it in evict_folios(). Since the
> > type to isolate is unknown until isolation and isolate_folios() may
> > fall back to the other type, check all evictable types with
> > for_each_evictable_type() and throttle if any of them has too many
> > isolated folios.
>
> I feel this is unlikely to work. MGLRU behaves quite differently from the
> active/inactive LRU, and its `NR_INACTIVE_FILE` accounting is also very
> different.
>
> With the active/inactive LRU, shrink_inactive_list() ensures that we
> always have an inactive list with pages available for reclaim. With MGLRU,
> however, a generation can legitimately point to an empty list, so this
> assumption does not hold.
>
> try_to_inc_min_seq:
>
>          /* see the comment on lru_gen_folio */
>          if (swappiness && swappiness <= MAX_SWAPPINESS) {
>                  unsigned long seq = lrugen->max_seq - MIN_NR_GENS;
>
>                  if (min_seq[LRU_GEN_ANON] > seq && min_seq[LRU_GEN_FILE] < seq)
>                          min_seq[LRU_GEN_ANON] = seq;
>                  else if (min_seq[LRU_GEN_FILE] > seq &&
> min_seq[LRU_GEN_ANON] < seq)
>                          min_seq[LRU_GEN_FILE] = seq;
>          }
>
> At that point, we have no inactive pages for the type, so the
> throttle will take effect when the following condition is true:
>
> too_many = isolated > inactive;
>
> With MGLRU, however, we can still fall back to the other type even
> when there are no inactive pages for the current type.

Yes, that's a valid concern. So I think we can check the isolation of 
both types for MGLRU to avoid this case:

static bool check_need_throttle()
{
        bool need_throttle = true;

        for_each_evictable_type(i, swappiness) {
                if (!too_many_isolated(pgdat, i, sc))
                        need_throttle = false;
        }

        return need_throttle;
}

In evict_folios():
	......
	while (unlikely(check_need_throttle())) {
                if (stalled)
                        return 0;

                /* wait a bit for the reclaimer. */
                stalled = true;
                reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED);

                /* We are about to die and free our memory. Return now. */
                if (fatal_signal_pending(current))
                        return SWAP_CLUSTER_MAX;
        }

> BTW, if we are hitting isolated > inactive with MGLRU, it probably
> means the generations are quite imbalanced—we are running out of
> reclaimable generations. In that case, we may actually want
> reclamation to proceed with aging instead.

The typical 'isolated > inactive' case is that we've tried our best with 
aging, but cold pages production can't keep up with isolation speed, 
especially under concurrent reclaim from multiple processes. In this 
case, I think throttling is reasonable.

> > If a fatal signal is pending, fake reclaim progress the same way the
> > legacy path does, so the dying task exits reclaim quickly instead of
> > being held in the throttle.
> >
> > Signed-off-by: Hui Zhu <zhuhui@xxxxxxxxxx>
> > ---
> >   mm/vmscan.c | 71 ++++++++++++++++++++++++++++++++++++++++++++---------
> >   1 file changed, 59 insertions(+), 12 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 98226bb021f3..6fe8824430ac 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -1819,7 +1819,7 @@ bool folio_isolate_lru(struct folio *folio)
> >    * the LRU list will go small and be scanned faster than necessary, leading to
> >    * unnecessary swapping, thrashing and OOM.
> >    */
> > -static bool too_many_isolated(struct pglist_data *pgdat, int file,
> > +static bool too_many_isolated(struct pglist_data *pgdat, bool file,
> >                  struct scan_control *sc)
> >   {
> >          unsigned long inactive, isolated;
> > @@ -1856,6 +1856,37 @@ static bool too_many_isolated(struct pglist_data *pgdat, int file,
> >          return too_many;
> >   }
> >
> > +/*
> > + * Throttle reclaim if too many isolated folios are piling up. If this makes
> > + * no progress, the caller is probably looping on unevictable folios, so give
> > + * up. Returns true to tell the caller to stop reclaiming, and sets @fatal
> > + * if the task received a fatal signal while waiting, so that the caller can
> > + * bail out faster.
> > + */
> > +static bool throttle_is_throttled(struct pglist_data *pgdat, bool file,
> > +                                 struct scan_control *sc, bool *fatal)
> > +{
> > +       bool stalled = false;
> > +
> > +       *fatal = false;
>
> TBH, I find the name quite weird :-)

Yes, that is not what I meant. :) What I mean is to use a readable 
variable to return instead of 'true' or 'false':

static bool throttle_isolated(struct pglist_data *pgdat, bool file,
                struct scan_control *sc, bool *fatal)
{
        bool stalled = false;

        *fatal = false;
        while (unlikely(too_many_isolated(pgdat, file, sc))) {
                if (stalled)
                        return stalled;

                /* wait a bit for the reclaimer. */
                stalled = true;
                reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED);

                /* We are about to die and free our memory. Return now. */
                if (fatal_signal_pending(current)) {
                        *fatal = true;
                        return stalled;
                }
        }

        return stalled;
}