Re: [PATCH] mm: fix draining remote pageset

From: Vlastimil Babka <vbabka@suse.cz>
To: "Lameter, Christopher" <cl@os.amperecomputing.com>,
	Michal Hocko <mhocko@suse.com>
Cc: "Huang, Ying" <ying.huang@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Mel Gorman <mgorman@techsingularity.net>
Subject: Re: [PATCH] mm: fix draining remote pageset
Date: Tue, 5 Sep 2023 18:52:29 +0200	[thread overview]
Message-ID: <703e284d-186a-5699-f06c-761e51115ae0@suse.cz> (raw)
In-Reply-To: <94b0e0c6-a626-46a1-e746-a336d20cdc08@os.amperecomputing.com>

On 8/25/23 19:06, Lameter, Christopher wrote:
> On Tue, 22 Aug 2023, Michal Hocko wrote:
> 
>> Yes, this doesn't really show any actual correctness problem so I do not
>> think this is sufficient to change the code. You would need to show that
>> the existing behavior is actively harmful.
> 
> Having some pages from a remote NUMA node stuck in a pcp somewhere is 
> making that memory unusable. It is usually rate that these remote pages 
> are needed again and so they may remain there for a long time if the 
> situation is right.
> 
> And he is right that the intended behavior of freeing the remote pages 
> has been disabled by the patch.
> 
> So I think there is sufficient rationale to apply these fixes.

I wonder if this the optimum way to handle the NOHZ case? IIUC there we use
quiet_vmstat() to call refresh_cpu_vm_stats(). I'd expect if there were
pending remote pages to flush, it would be best to do it immediately, and
not keep a worker being requeued and only do that after the pcp->expires
goes zero.

However quiet_vmstat() even calls the refresh with do_pagesets == false. Why
do we even refresh the stats at that moment if the delayed update is pending
anyway? And could we maybe make sure that in that case the flush is done on
the first delayed update in that case and not expiring like this?