mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] mm: fix two allocator races with zonelist and cpuset updates
@ 2026-10-14 22:24 Gregory Price (Meta)
  2026-10-14 22:24 ` [PATCH 1/2] mm/page_alloc: detect zonelist rebuilds from memory hot-add Gregory Price (Meta)
  2026-10-14 22:24 ` [PATCH 2/2] cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask Gregory Price (Meta)
  0 siblings, 2 replies; 7+ messages in thread
From: Gregory Price (Meta) @ 2026-10-14 22:24 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, kernel-team, longman, ridong.chen, tj, hannes,
	mkoutny, akpm, vbabka, surenb, mhocko, brendan.jackman, ziy,
	david, mgorman

Two small fixes for lockless allocator checks that can act on an
inconsistent view of state another CPU is updating.

Neither of these are stable-worthy.

Patch 1: the zonelist_update_seq retry is compiled out without
CONFIG_MEMORY_HOTREMOVE, but memory hot-add can also cause the race.
A __GFP_THISNODE allocation can observe this and fail to allocate.

Patch 2: cpuset_nodemask_valid_mems_allowed() compares a mempolicy
nodemask with mems_allowed without mems_allowed_seq. A concurrent
cpuset update can cause a spurious empty intersection and end up
relaxing an MPOL_BIND needlessly. This only covers task policies.

Reproducers
===========

Both races were reproduced on an unmodified kernel 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, CONFIG_MEMORY_HOTREMOVE=n).

The windows are a few instructions wide, so I used a test module to
widens them with a kprobe placed on a single instruction inside the
window.  This basically emulates an interrupt or vCPU preemption.

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 (cpuset):

  Put a task in a cpuset with mems={0}
  set_mempolicy(2) the task to MPOL_BIND={0}
  Run the task on node2 (yes this is a little flagrant).
  
  Fill node 0 with HugeTLB pages to make allocations fail.
  Stall between the two loads in cpuset_nodemask_valid_mems_allowed().

  While stalled, change cpuset.mems={1,2}, this rebinds the task
  mempolicy to MPOL_BIND={1}.

  The check compares the old mems_allowed (0) with the new policy (1),
  finds no overlap, and drops the nodemask.

  Before: the allocation lands on node 2, 3/3 runs.
  After:  the allocation lands on node 1, 3/3 runs.

Gregory Price (Meta) (2):
  mm/page_alloc: detect zonelist rebuilds from memory hot-add
  cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask

 kernel/cgroup/cpuset.c | 13 ++++++++++++-
 mm/page_alloc.c        |  4 ++--
 2 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] mm/page_alloc: detect zonelist rebuilds from memory hot-add
  2026-10-14 22:24 [PATCH 0/2] mm: fix two allocator races with zonelist and cpuset updates Gregory Price (Meta)
@ 2026-10-14 22:24 ` Gregory Price (Meta)
  2026-10-15  7:51   ` Vlastimil Babka (SUSE)
  2026-10-14 22:24 ` [PATCH 2/2] cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask Gregory Price (Meta)
  1 sibling, 1 reply; 7+ messages in thread
From: Gregory Price (Meta) @ 2026-10-14 22:24 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, kernel-team, longman, ridong.chen, tj, hannes,
	mkoutny, akpm, vbabka, surenb, mhocko, brendan.jackman, ziy,
	david, mgorman

The zonelist_update_seq retry was limited to CONFIG_MEMORY_HOTREMOVE,
presumably because only hot-remove takes zones out of a zonelist.

However, build_zonerefs_node() rewrites the zonelists in place, so
onlining the first block of a zone shifts the entries that follow it
in every zonelist containing that zone.  This can cause a spurious
allocation failure for a __GFP_THISNODE caller.

For a node with only ZONE_NORMAL, onlining its first ZONE_MOVABLE block
rewrites the node's ZONELIST_NOFALLBACK list as:

  [NORMAL, NULL] -> [MOVABLE, NULL] -> [MOVABLE, NORMAL, NULL]

A concurrent GFP_KERNEL | __GFP_THISNODE allocation can observe the
intermediate list, skip ZONE_MOVABLE, and find no eligible zone. With
CONFIG_MEMORY_HOTREMOVE=n, check_retry_zonelist() returns 0 and the
allocation fails even though the node has free ZONE_NORMAL memory.

Gate the sequence checks on CONFIG_MEMORY_HOTPLUG, which covers every
configuration that rebuilds zonelists after boot.

Fixes: 3d36424b3b58 ("mm/page_alloc: fix race condition between build_all_zonelists and page allocation")
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_alloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 7682aecc2c075..81db19f2ab551 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4425,7 +4425,7 @@ static DEFINE_SEQLOCK(zonelist_update_seq);
 
 static unsigned int zonelist_iter_begin(void)
 {
-	if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
+	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
 		return read_seqbegin(&zonelist_update_seq);
 
 	return 0;
@@ -4433,7 +4433,7 @@ static unsigned int zonelist_iter_begin(void)
 
 static unsigned int check_retry_zonelist(unsigned int seq)
 {
-	if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
+	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
 		return read_seqretry(&zonelist_update_seq, seq);
 
 	return seq;
-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask
  2026-10-14 22:24 [PATCH 0/2] mm: fix two allocator races with zonelist and cpuset updates Gregory Price (Meta)
  2026-10-14 22:24 ` [PATCH 1/2] mm/page_alloc: detect zonelist rebuilds from memory hot-add Gregory Price (Meta)
@ 2026-10-14 22:24 ` Gregory Price (Meta)
  2026-10-15  7:59   ` Vlastimil Babka (SUSE)
  1 sibling, 1 reply; 7+ messages in thread
From: Gregory Price (Meta) @ 2026-10-14 22:24 UTC (permalink / raw)
  To: linux-mm, cgroups
  Cc: linux-kernel, kernel-team, longman, ridong.chen, tj, hannes,
	mkoutny, akpm, vbabka, surenb, mhocko, brendan.jackman, ziy,
	david, mgorman

A cpuset update changes the task's mems_allowed and rebinds its
mempolicy in separate steps under mems_allowed_seq, but the check
reads both without it.  This can cause a spurious numa placement
miss under some cpuset/mempolicy combinations.

A reader that loads one mask before the update and the other after
it can see a spurious empty intersection and drop the nodemask.

For example, a task with cpuset.mems={0} and MPOL_BIND={0}.  If the 
cpuset moves to mems={1,2}, the policy is rebound to {1}.

The allocator can race and compare the old mems_allowed {0} against
the new policy {1} - which will result in a dropped nodemask and a
misplaced allocation on node 2.

Simply retry the intersection until mems_allowed_seq is stable.

This covers the task policy, which is rebound under mems_allowed_seq.
VMA policies are rebound afterwards by mpol_rebind_mm(), outside
mems_allowed_seq, and are not covered by this change.

Fixes: cc9a6c877661 ("cpuset: mm: reduce large amounts of memory barrier related damage v3")
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 kernel/cgroup/cpuset.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 3f52717c19654..8ee239d5c6b48 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4354,10 +4354,21 @@ nodemask_t cpuset_mems_allowed(struct task_struct *tsk)
  * @nodemask: the nodemask to be checked
  *
  * Are any of the nodes in the nodemask allowed in current->mems_allowed?
+ *
+ * A cpuset update rebinds the task's mempolicy and changes mems_allowed in
+ * separate steps, so retry if an update raced with the check.
  */
 int cpuset_nodemask_valid_mems_allowed(const nodemask_t *nodemask)
 {
-	return nodes_intersects(*nodemask, current->mems_allowed);
+	unsigned int seq;
+	int ret;
+
+	do {
+		seq = read_mems_allowed_begin();
+		ret = nodes_intersects(*nodemask, current->mems_allowed);
+	} while (read_mems_allowed_retry(seq));
+
+	return ret;
 }
 
 /*
-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] mm/page_alloc: detect zonelist rebuilds from memory hot-add
  2026-10-14 22:24 ` [PATCH 1/2] mm/page_alloc: detect zonelist rebuilds from memory hot-add Gregory Price (Meta)
@ 2026-10-15  7:51   ` Vlastimil Babka (SUSE)
  0 siblings, 0 replies; 7+ messages in thread
From: Vlastimil Babka (SUSE) @ 2026-10-15  7:51 UTC (permalink / raw)
  To: Gregory Price (Meta), linux-mm, cgroups
  Cc: linux-kernel, kernel-team, longman, ridong.chen, tj, hannes,
	mkoutny, akpm, surenb, mhocko, brendan.jackman, ziy, david,
	mgorman

On 10/15/26 00:24, Gregory Price (Meta) wrote:
> The zonelist_update_seq retry was limited to CONFIG_MEMORY_HOTREMOVE,
> presumably because only hot-remove takes zones out of a zonelist.
> 
> However, build_zonerefs_node() rewrites the zonelists in place, so
> onlining the first block of a zone shifts the entries that follow it
> in every zonelist containing that zone.  This can cause a spurious
> allocation failure for a __GFP_THISNODE caller.
> 
> For a node with only ZONE_NORMAL, onlining its first ZONE_MOVABLE block
> rewrites the node's ZONELIST_NOFALLBACK list as:
> 
>   [NORMAL, NULL] -> [MOVABLE, NULL] -> [MOVABLE, NORMAL, NULL]
> 
> A concurrent GFP_KERNEL | __GFP_THISNODE allocation can observe the
> intermediate list, skip ZONE_MOVABLE, and find no eligible zone. With
> CONFIG_MEMORY_HOTREMOVE=n, check_retry_zonelist() returns 0 and the
> allocation fails even though the node has free ZONE_NORMAL memory.
> 
> Gate the sequence checks on CONFIG_MEMORY_HOTPLUG, which covers every
> configuration that rebuilds zonelists after boot.
> 
> Fixes: 3d36424b3b58 ("mm/page_alloc: fix race condition between build_all_zonelists and page allocation")
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/page_alloc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 7682aecc2c075..81db19f2ab551 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4425,7 +4425,7 @@ static DEFINE_SEQLOCK(zonelist_update_seq);
>  
>  static unsigned int zonelist_iter_begin(void)
>  {
> -	if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
> +	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
>  		return read_seqbegin(&zonelist_update_seq);
>  
>  	return 0;
> @@ -4433,7 +4433,7 @@ static unsigned int zonelist_iter_begin(void)
>  
>  static unsigned int check_retry_zonelist(unsigned int seq)
>  {
> -	if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
> +	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
>  		return read_seqretry(&zonelist_update_seq, seq);
>  
>  	return seq;


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask
  2026-10-14 22:24 ` [PATCH 2/2] cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask Gregory Price (Meta)
@ 2026-10-15  7:59   ` Vlastimil Babka (SUSE)
  2026-10-15 13:38     ` Gregory Price
  0 siblings, 1 reply; 7+ messages in thread
From: Vlastimil Babka (SUSE) @ 2026-10-15  7:59 UTC (permalink / raw)
  To: Gregory Price (Meta), linux-mm, cgroups
  Cc: linux-kernel, kernel-team, longman, ridong.chen, tj, hannes,
	mkoutny, akpm, surenb, mhocko, brendan.jackman, ziy, david,
	mgorman

On 10/15/26 00:24, Gregory Price (Meta) wrote:
> A cpuset update changes the task's mems_allowed and rebinds its
> mempolicy in separate steps under mems_allowed_seq, but the check
> reads both without it.  This can cause a spurious numa placement
> miss under some cpuset/mempolicy combinations.
> 
> A reader that loads one mask before the update and the other after
> it can see a spurious empty intersection and drop the nodemask.
> 
> For example, a task with cpuset.mems={0} and MPOL_BIND={0}.  If the 
> cpuset moves to mems={1,2}, the policy is rebound to {1}.
> 
> The allocator can race and compare the old mems_allowed {0} against
> the new policy {1} - which will result in a dropped nodemask and a
> misplaced allocation on node 2.

> Simply retry the intersection until mems_allowed_seq is stable.
> 
> This covers the task policy, which is rebound under mems_allowed_seq.
> VMA policies are rebound afterwards by mpol_rebind_mm(), outside
> mems_allowed_seq, and are not covered by this change.
> 
> Fixes: cc9a6c877661 ("cpuset: mm: reduce large amounts of memory barrier related damage v3")
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

Should the comment and/or code in check_retry_cpuset() be updated as well?

> ---
>  kernel/cgroup/cpuset.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 3f52717c19654..8ee239d5c6b48 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -4354,10 +4354,21 @@ nodemask_t cpuset_mems_allowed(struct task_struct *tsk)
>   * @nodemask: the nodemask to be checked
>   *
>   * Are any of the nodes in the nodemask allowed in current->mems_allowed?
> + *
> + * A cpuset update rebinds the task's mempolicy and changes mems_allowed in
> + * separate steps, so retry if an update raced with the check.
>   */
>  int cpuset_nodemask_valid_mems_allowed(const nodemask_t *nodemask)
>  {
> -	return nodes_intersects(*nodemask, current->mems_allowed);
> +	unsigned int seq;
> +	int ret;
> +
> +	do {
> +		seq = read_mems_allowed_begin();
> +		ret = nodes_intersects(*nodemask, current->mems_allowed);
> +	} while (read_mems_allowed_retry(seq));
> +
> +	return ret;
>  }
>  
>  /*


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask
  2026-10-15  7:59   ` Vlastimil Babka (SUSE)
@ 2026-10-15 13:38     ` Gregory Price
  2026-10-15 13:49       ` Vlastimil Babka (SUSE)
  0 siblings, 1 reply; 7+ messages in thread
From: Gregory Price @ 2026-10-15 13:38 UTC (permalink / raw)
  To: Vlastimil Babka (SUSE)
  Cc: linux-mm, cgroups, linux-kernel, kernel-team, longman,
	ridong.chen, tj, hannes, mkoutny, akpm, surenb, mhocko,
	brendan.jackman, ziy, david, mgorman

On Thu, Oct 15, 2026 at 09:59:31AM +0200, Vlastimil Babka (SUSE) wrote:
> On 10/15/26 00:24, Gregory Price (Meta) wrote:
> > A cpuset update changes the task's mems_allowed and rebinds its
> > mempolicy in separate steps under mems_allowed_seq, but the check
> > reads both without it.  This can cause a spurious numa placement
> > miss under some cpuset/mempolicy combinations.
> > 
> > A reader that loads one mask before the update and the other after
> > it can see a spurious empty intersection and drop the nodemask.
> > 
> > For example, a task with cpuset.mems={0} and MPOL_BIND={0}.  If the 
> > cpuset moves to mems={1,2}, the policy is rebound to {1}.
> > 
> > The allocator can race and compare the old mems_allowed {0} against
> > the new policy {1} - which will result in a dropped nodemask and a
> > misplaced allocation on node 2.
> 
> > Simply retry the intersection until mems_allowed_seq is stable.
> > 
> > This covers the task policy, which is rebound under mems_allowed_seq.
> > VMA policies are rebound afterwards by mpol_rebind_mm(), outside
> > mems_allowed_seq, and are not covered by this change.
> > 
> > Fixes: cc9a6c877661 ("cpuset: mm: reduce large amounts of memory barrier related damage v3")
> > Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
> 
> Should the comment and/or code in check_retry_cpuset() be updated as well?
> 

No I don't think so. We're checking different things.

The loop here makes sure nodemask and mems_allowed are stable when
compared so we don't erroneously drop the nodemask on a race.

The cookie-check in check_retry_cpuset() determines whether the entire
allocation should restart.  It runs after the nodemask would already
have been dropped, so it can't catch this race.

~Gregory

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask
  2026-10-15 13:38     ` Gregory Price
@ 2026-10-15 13:49       ` Vlastimil Babka (SUSE)
  0 siblings, 0 replies; 7+ messages in thread
From: Vlastimil Babka (SUSE) @ 2026-10-15 13:49 UTC (permalink / raw)
  To: Gregory Price
  Cc: linux-mm, cgroups, linux-kernel, kernel-team, longman,
	ridong.chen, tj, hannes, mkoutny, akpm, surenb, mhocko,
	brendan.jackman, ziy, david, mgorman

On 10/15/26 15:38, Gregory Price wrote:
> On Thu, Oct 15, 2026 at 09:59:31AM +0200, Vlastimil Babka (SUSE) wrote:
>> On 10/15/26 00:24, Gregory Price (Meta) wrote:
>> > A cpuset update changes the task's mems_allowed and rebinds its
>> > mempolicy in separate steps under mems_allowed_seq, but the check
>> > reads both without it.  This can cause a spurious numa placement
>> > miss under some cpuset/mempolicy combinations.
>> > 
>> > A reader that loads one mask before the update and the other after
>> > it can see a spurious empty intersection and drop the nodemask.
>> > 
>> > For example, a task with cpuset.mems={0} and MPOL_BIND={0}.  If the 
>> > cpuset moves to mems={1,2}, the policy is rebound to {1}.
>> > 
>> > The allocator can race and compare the old mems_allowed {0} against
>> > the new policy {1} - which will result in a dropped nodemask and a
>> > misplaced allocation on node 2.
>> 
>> > Simply retry the intersection until mems_allowed_seq is stable.
>> > 
>> > This covers the task policy, which is rebound under mems_allowed_seq.
>> > VMA policies are rebound afterwards by mpol_rebind_mm(), outside
>> > mems_allowed_seq, and are not covered by this change.
>> > 
>> > Fixes: cc9a6c877661 ("cpuset: mm: reduce large amounts of memory barrier related damage v3")
>> > Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
>> 
>> Should the comment and/or code in check_retry_cpuset() be updated as well?
>> 
> 
> No I don't think so. We're checking different things.
> 
> The loop here makes sure nodemask and mems_allowed are stable when
> compared so we don't erroneously drop the nodemask on a race.
> 
> The cookie-check in check_retry_cpuset() determines whether the entire
> allocation should restart.  It runs after the nodemask would already
> have been dropped, so it can't catch this race.

Ah, right.

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ~Gregory


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-15 13:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-14 22:24 [PATCH 0/2] mm: fix two allocator races with zonelist and cpuset updates Gregory Price (Meta)
2026-10-14 22:24 ` [PATCH 1/2] mm/page_alloc: detect zonelist rebuilds from memory hot-add Gregory Price (Meta)
2026-10-15  7:51   ` Vlastimil Babka (SUSE)
2026-10-14 22:24 ` [PATCH 2/2] cgroup/cpuset: use mems_allowed_seq when checking a policy nodemask Gregory Price (Meta)
2026-10-15  7:59   ` Vlastimil Babka (SUSE)
2026-10-15 13:38     ` Gregory Price
2026-10-15 13:49       ` Vlastimil Babka (SUSE)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®