All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Joshua Hahn <joshua.hahnjy@gmail.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	 Gregory Price <gourry@gourry.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	 David Hildenbrand <david@kernel.org>,
	Vlastimil Babka <vbabka@kernel.org>,
	 "Liam R. Howlett" <liam@infradead.org>,
	Mike Rapoport <rppt@kernel.org>,
	 Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>,
	 Brendan Jackman <brendan.jackman@linux.dev>,
	Zi Yan <ziy@nvidia.com>, Kairui Song <kasong@tencent.com>,
	 Qi Zheng <qi.zheng@linux.dev>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	 Barry Song <baohua@kernel.org>,
	Axel Rasmussen <axelrasmussen@google.com>,
	 Yuanchu Xie <yuanchu@google.com>, Wei Xu <weixugc@google.com>,
	Baoquan He <baoquan.he@linux.dev>,
	 Baolin Wang <baolin.wang@linux.alibaba.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	 kernel-team@meta.com
Subject: Re: [PATCH 2/2] mm/page_alloc: only skip kswapd nodes below their kswapd watermark
Date: Fri, 6 Nov 2026 17:50:07 +0000	[thread overview]
Message-ID: <au4SKZKfJoE0ZgAw@gremlin> (raw)
In-Reply-To: <20261106172620.318113-2-joshua.hahnjy@gmail.com>

On Fri, Nov 06, 2026 at 09:26:19AM -0800, Joshua Hahn wrote:
> Commit 19c5fb83f2a4 ("mm: page_alloc: avoid kswapd thrashing due to
> NUMA restrictions") makes get_page_from_freelist() skip any node whose
> kswapd is active to avoid piling on an already-pressured node that is
> constantly churning to make free space that is immediately consumed.
>
> But kswapd can also be woken up for order 9 requests with THP defrag
> set to defer. This means that a fragmented node with plenty of free
> memory will have a kswapd thread continuously spinning and preventing
> order 0 pages from allocating on the node. This causes the opposite
> problem of piling on pressure on all other nodes.
>
> This behavior was observed on some Meta production workloads running on
> 2-NUMA systems, where processes with a preference for node0 allocations
> continuously allocated order 0 pages on node1, despite node0 having
> hundreds of GB of free memory.
>
> To prevent piling pressure on any one node, only skip a node during
> page allocation if the zone also fails the watermark kswapd is
> reclaiming it up to. This is the high watermark, or the promo watermark
> if NUMA balancing memory tiering is enabled. Make sure to mask the
> reserve flags so that __GFP_HIGH / atomic requests in the slowpath
> do not get a relaxed version of the check.
>
> This preserves the existing fix to prevent piling memory on the local
> node, while also preventing memory pileups on non-local nodes when
> the local node has memory.
>
> On a 2-NUMA VM with a 60G node0 and a 20G node1, with THP enabled=always
> and defrag=defer, I ran a process that prefers allocating on node0.
> With a fragmented node0:
> - Before the patch, 66% of faults were served from node0
> - After the patch, 100% of faults were served from node0
>
> Suggested-by: Daniel Hodges <hodgesd@meta.com>
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> ---
>  mm/page_alloc.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5423ff4805f84..8e8a2c8d6e6a4 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3882,14 +3882,18 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  		}
>
>  		/*
> -		 * If kswapd is already active on a node, keep looking
> -		 * for other nodes that might be idle. This can happen
> -		 * if another process has NUMA bindings and is causing
> -		 * kswapd wakeups on only some nodes. Avoid accidental
> -		 * "node_reclaim_mode"-like behavior in this case.
> +		 * If kswapd is already active on a node and the zone is below
> +		 * its kswapd watermark for this request, keep looking for other
> +		 * nodes that might be idle. This can happen if another process
> +		 * has NUMA bindings and is causing kswapd wakeups on only some
> +		 * nodes. Avoid accidental "node_reclaim_mode" behavior.
>  		 */
>  		if (skip_kswapd_nodes &&
> -		    !waitqueue_active(&zone->zone_pgdat->kswapd_wait)) {
> +		    !waitqueue_active(&zone->zone_pgdat->kswapd_wait) &&
> +		    !zone_watermark_fast(zone, order, kswapd_wmark_pages(zone),
> +					 ac->highest_zoneidx,
> +					 alloc_flags & ~ALLOC_RESERVES,
> +					 gfp_mask)) {

I know this code is hardly the cleanest, but you're now adding a 3rd condition
to a branch that was already complicated enough to have a massive comment and
duplicating a block of code that already dupliclated twice here.

You're also then going against the convention of

	mark = ...
	if (... !zone_watermark_fast(zone, order, mark,
					 ac->highest_zoneidx, alloc_flags,
					 gfp_mask)) {

(Obviously yours varies on alloc_flags too)

And also inlining the mark value.

Could you clean this up a little given this isn't a fix?

In general extending functionality should pay down some technical debt if
possible, it's ok to add a preceding patch in the series to do something like
that.

>  			skipped_kswapd_nodes = true;
>  			continue;
>  		}
> --
> 2.53.0-Meta
>

--
Cheers, Lorenzo


  reply	other threads:[~2026-11-06 17:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-06 17:26 [PATCH 1/2] mm/vmscan: consolidate promo vs. high watermark checks Joshua Hahn
2026-11-06 17:26 ` [PATCH 2/2] mm/page_alloc: only skip kswapd nodes below their kswapd watermark Joshua Hahn
2026-11-06 17:50   ` Lorenzo Stoakes (ARM) [this message]
2026-11-06 18:35     ` Joshua Hahn
2026-11-07 17:26   ` sashiko-bot
2026-11-09 17:27     ` Joshua Hahn
2026-11-09 17:38       ` Lorenzo Stoakes (ARM)
2026-11-06 17:42 ` [PATCH 1/2] mm/vmscan: consolidate promo vs. high watermark checks Lorenzo Stoakes (ARM)
2026-11-06 17:50   ` Joshua Hahn
2026-11-07 17:26 ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=au4SKZKfJoE0ZgAw@gremlin \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=baoquan.he@linux.dev \
    --cc=brendan.jackman@linux.dev \
    --cc=david@kernel.org \
    --cc=gourry@gourry.net \
    --cc=hannes@cmpxchg.org \
    --cc=joshua.hahnjy@gmail.com \
    --cc=kasong@tencent.com \
    --cc=kernel-team@meta.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=qi.zheng@linux.dev \
    --cc=rppt@kernel.org \
    --cc=shakeel.butt@linux.dev \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    --cc=weixugc@google.com \
    --cc=yuanchu@google.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.