Re: [PATCH v3 0/8] mm: fix allocator races with memory hotplug

["David Hildenbrand (Arm)" <david@kernel.org>]

On 10/21/26 04:39, Gregory Price (Meta) wrote:

Usual reminder to slow down a bit :)

We have

	Documentation/process/submitting-patches.rst

Saying

	"Wait for a minimum of one week before resubmitting or pinging reviewers
	 - possibly longer during busy times like merge windows.

A lot of this here was broken forever, so there is no need to rush.

(thanks for the fixes ;) )

> Eight fixes for allocator paths that can act on state another CPU is
> updating: zonelists rebuilt by memory hotplug, and the pagesets of a
> zone that offlining empties.
> 
> I've packaged these together because they're all related to hotplug.
> 
> *** Backport note - Patches 5-7 build on patch 1: without it, their
>     retry is compiled out when CONFIG_MEMORY_HOTREMOVE=n.
> 
> Most of these race conditions stem from zonelists being re-ordered
> in-place at runtime during hotplug.  The result is a mixture of failed
> allocations, NULL dereferences, incorrect placement (GFP_KERNEL in
> ZONE_MOVABLE), node misplacement, or incorrectly allowing hugetlb /
> contig allocations on a node/zone otherwise not eligible.
> 
> Most of these are incredibly narrow race conditions, but since the most
> likely impacted machines are VMs with memory hotplug, the narrow
> condition can more easily occur when a vCPU takes a long interrupt / VM
> exit of some kind.  I've made all of these race conditions
> deterministics with a kprobe reproduction module described below.
> 
> Reproducers
> ===========
> 
> All eight races were reproduced on unmodified source in a QEMU guest
> with three NUMA nodes:
>    node 0: 3G, CPUs 0-1
>    node 1: 1G ZONE_NORMAL, CPU 2
>    node 2: 1G ZONE_NORMAL, CPU 3
> 
> And a 512M pc-dimm cold-plugged on node 1, left offline at boot
> (CONFIG_ACPI_HOTPLUG_MEMORY=y).
> 
> Patch 1 needs CONFIG_MEMORY_HOTREMOVE=n to show the failure
> Patch 3 needs mm/mmzone.o built with -fno-inline -fno-ipa-sra,
>         without which the two reads are merged.
> Patch 4 needs the reclaim_retry_zone tracepoint enabled.
> Patch 7's hugetlb case needs a 64G guest (node 1: 4G) so that one
>         memory block is 1G, with a 2G pc-dimm on node 1.
> 
> The windows are a few instructions wide, so I used a test module to
> widen them with kprobes that stall one CPU inside the window. This
> emulates an interrupt or vCPU preemption.  The vCPU preemption is the
> more likely event to cause this at large scale with memory hotplug.
> 
> Patch 1 (zonelist):
> 
>   Online node 1's first memory block as ZONE_MOVABLE, which rebuilds
>   node 1's zonelists.
> 
>   Stall the rebuild after it writes ZONE_MOVABLE into the first entry
>   but before it writes ZONE_NORMAL into the second.
> 
>   On another CPU, allocate GFP_KERNEL | __GFP_THISNODE from node 1.
> 
>   Before: the allocation fails, 3/3 runs.
>   After:  the allocation waits for the rebuild and succeeds, 3/3 runs.
> 
> Patch 2 (preferred zone):
> 
>   Online node 1's block as ZONE_MOVABLE, so node 1's local zonelist is
>   [MOVABLE, NORMAL].
> 
>   On another CPU, allocate GFP_KERNEL | __GFP_THISNODE from node 1,
>   whose preferred slot is NORMAL, and stall it after it loads the zone.
> 
>   Offline the block, which rebuilds the list as [NORMAL] and moves the
>   terminator into the preferred slot.
> 
>   Before: NULL dereference in get_page_from_freelist(), 3/3 runs.
>   After:  the allocation succeeds from ZONE_NORMAL, 3/3 runs.
> 
> Patch 3 (slot reread):
> 
>   Online node 1's block as ZONE_MOVABLE, so node 1's local zonelist is
>   [MOVABLE, NORMAL].
> 
>   On another CPU, allocate GFP_KERNEL | __GFP_THISNODE from node 1
>   with nodemask {1}, and stall it after __next_zones_zonelist() tests
>   the NORMAL slot for NULL but before it reads the slot again.
> 
>   Offline the block, which rebuilds the list as [NORMAL] and moves the
>   terminator into that slot.
> 
>   Before: NULL dereference in zone_to_nid(), 3/3 runs.
>   After:  the allocation succeeds from ZONE_NORMAL, 3/3 runs.
> 
> Patch 4 (tracepoint):
> 
>   Online node 1's block as ZONE_MOVABLE, so node 1's local zonelist is
>   [MOVABLE, NORMAL], and enable the reclaim_retry_zone tracepoint.
> 
>   On another CPU, allocate GFP_KERNEL | __GFP_THISNODE from node 1, with
>   its get_page_from_freelist() calls failed so that the slowpath reaches
>   should_reclaim_retry(), and stall it in zone_reclaimable_pages() for
>   node 1's NORMAL zone.
> 
>   Offline the block, which rebuilds the list as [NORMAL] and moves the
>   terminator into that slot.
> 
>   Before: NULL dereference in the tracepoint, 3/3 runs. With the
>           tracepoint disabled, no oops (1 run).
>   After:  the allocation succeeds from ZONE_NORMAL, 3/3 runs.
> 
> Patch 5 (zonelist slots):
> 
>   On one CPU, allocate GFP_KERNEL with a nodemask, and stall it after
>   the iterator approves its first zonelist slot but before the
>   allocator loads the zone from that slot.
> 
>   Online node 1's first memory block as ZONE_MOVABLE, which inserts
>   ZONE_MOVABLE at the front of node 1's zonelists.
> 
>   This was run for single-page and bulk allocations, and for a
>   single-page allocation whose fast path is made to fail so that the
>   slowpath's walk races.
> 
>   Before: nodemask {1} gets a node 1 ZONE_MOVABLE page and nodemask
>           {2} a node 1 ZONE_NORMAL page, 18/18 runs.
>   After:  nodemask {1} gets node 1 ZONE_NORMAL and nodemask {2} gets
>           node 2 ZONE_NORMAL, 18/18 runs.
> 
>   The check adds 5 instructions to an order-0 allocation on x86,
>   counted with a hardware counter on a non-debug build.
> 
> Patch 6 (hugetlb):
> 
>   Reserve hugetlb pages on every node. On CPU 1, fault a hugetlb page
>   bound to node 2 with mbind(); the pool walk uses node 0's zonelist,
>   where node 2 follows node 1. Stall the walk after it approves
>   node 2's slot.
> 
>   Online node 1's first memory block as ZONE_MOVABLE, which shifts
>   node 1's ZONE_NORMAL into that slot.
> 
>   Before: the page comes from node 1, 3/3 runs.
>   After:  the page comes from node 2, 3/3 runs.
> 
> Patch 7 (alloc_contig):
> 
>   Write 1 to node 1's 1G nr_hugepages, and stall the gigantic page's
>   zonelist walk after it approves node 1's ZONE_NORMAL slot.
> 
>   Online node 1's first memory block as ZONE_MOVABLE, which inserts
>   ZONE_MOVABLE into that slot.
> 
>   Before: the 1G page is allocated from ZONE_MOVABLE, 3/3 runs.
>   After:  the 1G page is allocated from ZONE_NORMAL, 3/3 runs.
> 
>   Calling alloc_contig_pages() directly with GFP_KERNEL in the first
>   guest, as in patch 5, nodemask {1} gets a ZONE_MOVABLE range and
>   nodemask {2} a node 1 range before, 3/3 runs each, and neither after.
> 
> Patch 8 (pagesets):
> 
>   Online node 1's block as ZONE_MOVABLE, then offline it.
> 
>   Stall the offline before the zone empties, and stall the vmstat
>   worker on another CPU (kicked through /proc/sys/vm/stat_refresh) in
>   decay_pcp_high() for that zone. Let the offline free the pagesets,
>   reallocate the freed percpu area with a known value, then release
>   the worker.
> 
>   Before: decay_pcp_high() runs on the freed pageset, 6/6 runs. In
>           1/6 the area was reallocated before the worker resumed, and
>           decay_pcp_high() rewrote the new object.
>   After:  the pageset is not freed, 3/3 runs.
> 
> All eight reproducers also pass on the series tip, 3/3 runs each.
> 
> Changes in v3
> =============
> - Replaced v2 patch 2 (zonelist_zone_valid() recheck) with patch 5,
>   which checks zonelist_update_seq whenever an allocation succeeds
>   (Vlastimil). A raced attempt's page is discarded, so a stale preferred
>   zone can no longer affect the result (Sashiko).
> - Moved the NULL dereference fixes ahead of patch 5.
> - Added patches 6 and 7 for the hugetlb pool walk and
>   alloc_contig_pages(), which have the same slot shift.
> - Added patch 4 for the reclaim_retry_zone tracepoint's re-read.
> - Fixes: tags on patches 2-6 now name the stop_machine
>   removal that made rebuilds run concurrently with zonelist walks.
> 
> Link: https://lore.kernel.org/linux-mm/20261014222456.2318284-1-gourry@gourry.net/
> Link: https://lore.kernel.org/linux-mm/20261015210539.3243563-1-gourry@gourry.net/
> Link: https://lore.kernel.org/linux-mm/20261019172346.3344005-1-gourry@gourry.net/
> 
> Gregory Price (Meta) (8):
>   mm/page_alloc: detect zonelist rebuilds from memory hot-add
>   mm/page_alloc: snapshot the preferred zone against shrink rebuilds
>   mm/mmzone: read each zoneref slot once when filtering by nodemask
>   mm/page_alloc: pass the zone, not the zoneref, to reclaim_retry_zone
>   mm/page_alloc: retry allocations that raced with a zonelist rebuild
>   mm/hugetlb: retry the pool walk when the zonelist is rebuilt
>   mm/page_alloc: retry alloc_contig_pages() when the zonelist is rebuilt
>   mm/page_alloc: keep a zone's pagesets when it goes empty
> 
>  include/trace/events/oom.h |   8 +--
>  mm/hugetlb.c               |  13 +++--
>  mm/mmzone.c                |  11 ++--
>  mm/page_alloc.c            | 106 +++++++++++++++++++++++++++++--------
>  mm/page_alloc.h            |   3 ++
>  5 files changed, 107 insertions(+), 34 deletions(-)
> 


-- 
Cheers,

David