Re: [PATCH v4 1/1] mm: vmscan: Reduce throttling due to a failure to make progress

From: Hugh Dickins <hughd@google.com>
To: Mel Gorman <mgorman@techsingularity.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.com>,  Vlastimil Babka <vbabka@suse.cz>,
	Alexey Avramov <hakavlad@inbox.lv>,
	 Rik van Riel <riel@surriel.com>, Mike Galbraith <efault@gmx.de>,
	 Darrick Wong <djwong@kernel.org>,
	Shakeel Butt <shakeelb@google.com>,
	 regressions@lists.linux.dev,
	Linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	 Linux-MM <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 1/1] mm: vmscan: Reduce throttling due to a failure to make progress
Date: Wed, 8 Dec 2021 22:20:47 -0800 (PST)	[thread overview]
Message-ID: <df896d80-9972-5be-55c2-c5b0c7135d4b@google.com> (raw)
In-Reply-To: <20211202150614.22440-1-mgorman@techsingularity.net>

On Thu, 2 Dec 2021, Mel Gorman wrote:
...
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
...
> @@ -3478,14 +3520,18 @@ static void shrink_zones(struct zonelist *zonelist, struct scan_control *sc)
>  			/* need some check for avoid more shrink_zone() */
>  		}
>  
> +		if (!first_pgdat)
> +			first_pgdat = zone->zone_pgdat;
> +
>  		/* See comment about same check for global reclaim above */
>  		if (zone->zone_pgdat == last_pgdat)
>  			continue;
>  		last_pgdat = zone->zone_pgdat;
>  		shrink_node(zone->zone_pgdat, sc);
> -		consider_reclaim_throttle(zone->zone_pgdat, sc);
>  	}
>  
> +	consider_reclaim_throttle(first_pgdat, sc);

My tmpfs swapping load (tweaked to use huge pages more heavily than
in real life) is far from being a realistic load: but it was notably
slowed down by your throttling mods in 5.16-rc, and this patch makes
it well again - thanks.

But: it very quickly hit NULL pointer until I changed that last line to

	if (first_pgdat)
		consider_reclaim_throttle(first_pgdat, sc);

I've given no thought as to whether that is the correct fix,
or if first_pgdat should be set earlier in the loop above.

Hugh