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

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

On 8/17/26 3:11 PM, Hui Zhu 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_isolated() and reuse it in evict_folios(). The type to
> isolate is predicted with get_type_to_scan() since it is unknown
> until isolation.
> 
> 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 | 67 ++++++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 56 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index fdc45d7d8fba..886a53f563ab 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 false 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_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 false;
> +
> +		/* 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 false;
> +		}
> +	}
> +
> +	return true;

Returning ‘true’ or 'false' looks confusing to me, maybe use a readable 
variable 'is_throttled'?

> +}
> +
>   /*
>    * move_folios_to_lru() moves folios from private @list to appropriate LRU list.
>    *
> @@ -1992,19 +2023,14 @@ static unsigned long shrink_inactive_list(unsigned long nr_to_scan,
>   	bool file = is_file_lru(lru);
>   	enum node_stat_item item;
>   	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
> -	bool stalled = false;
> -
> -	while (unlikely(too_many_isolated(pgdat, file, sc))) {
> -		if (stalled)
> -			return 0;
> -
> -		/* wait a bit for the reclaimer. */
> -		stalled = true;
> -		reclaim_throttle(pgdat, VMSCAN_THROTTLE_ISOLATED);
> +	bool fatal;
>   
> +	if (!throttle_isolated(pgdat, file, sc, &fatal)) {
>   		/* We are about to die and free our memory. Return now. */
> -		if (fatal_signal_pending(current))
> +		if (fatal)
>   			return SWAP_CLUSTER_MAX;
> +
> +		return 0;
>   	}
>   
>   	lru_add_drain();
> @@ -4883,6 +4909,25 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   	bool skip_retry = false;
>   	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>   	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
> +	bool fatal;
> +
> +	/*
> +	 * The type to isolate is unknown until isolation, so predict it for
> +	 * the throttling check. isolate_folios() may still fall back to the
> +	 * other type, which is fine for this heuristic.
> +	 */
> +	type = get_type_to_scan(lruvec, swappiness);

Perhaps we could use 'for_each_evictable_type(i, swappiness)' to check 
each type's isolation?

> +	if (!throttle_isolated(pgdat, type, sc, &fatal)) {
> +		/*
> +		 * We are about to die and free our memory. Like the legacy
> +		 * path, pretend some pages were reclaimed so reclaim unwinds
> +		 * quickly instead of looping back into the throttle.
> +		 */
> +		if (fatal)
> +			sc->nr_reclaimed += SWAP_CLUSTER_MAX;
> +
> +		return 0;
> +	}
>   
>   	lruvec_lock_irq(lruvec);
>