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/[email protected]/
> Link:
> https://lore.kernel.org/linux-mm/[email protected]/
> Link:
> https://lore.kernel.org/linux-mm/[email protected]/
>
> 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