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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 8/19/26 6:06 AM, Barry Song wrote:
> On Tue, Aug 18, 2026 at 8:49 PM Hui Zhu <hui.zhu@linux.dev> wrote:
>>
>> From: Hui Zhu <zhuhui@kylinos.cn>
>>
>> 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@kylinos.cn>
>> ---
>>   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;
}