Re: [PATCH 2/2] mm/page_alloc: only skip kswapd nodes below their kswapd watermark

[Joshua Hahn]

On Fri, Nov 6, 2026 at 12:50 PM Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxx>
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
> > ---
> >  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?

Hi Lorenzo,

Thanks for taking a look at the patches! I think your requests are
valid here, let me fold that whole check into a helper function
including the comment block and hopefully that will make
get_page_from_freelist() a bit easier to read @_@. I also found an
unused gfp argument anyways, so I'll fold in some related cleanups and
send a new series.

I'll wait until tomorrow to send it out to see if other folks have
concerns with this. Thanks again, have a great day!
Joshua

> 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