mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online
@ 2026-06-27 20:22 Gregory Price
  2026-06-27 23:10 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Gregory Price @ 2026-06-27 20:22 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, kernel-team, rppt, akpm, vbabka, mgorman, hannes, stable

A per-node vmstat counter is pgdat->vm_stat[] plus per-cpu deltas.
A balanced counter can sit split as global=+N / per-cpu=-N.

The folds reconciling the split only walk online nodes, so when
try_offline_node() marks a node offline the per-cpu deltas are stranded.

A subsequent online resets the per-cpu area but not pgdat->vm_stat[],
orphaning the +N permanently.  All NR_VM_NODE_STAT_ITEMS are affected.

The existing code zeroes the per-cpu counters and causes a permanent
skew. Fold the stranded deltas instead, before the node rejoins the
online set. The node is not online yet and the hotplug lock is held,
so the remote access to per-cpu values is safe.

Discovered when node compaction hung for a nearly empty node, as the
math to determine throttling broke.  Reproduced by repeated memory
hotplug/unplug cycles on a node under pressure: NR_ISOLATED_ANON
ratchets up and never returns to zero.

Fixes: 75ef71840539 ("mm, vmstat: add infrastructure for per-node vmstats")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price <gourry@gourry.net>
---
 mm/mm_init.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index f5301d4de91a..c119f6f1497d 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1536,7 +1536,7 @@ void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
 {
 	int nid = pgdat->node_id;
 	enum zone_type z;
-	int cpu;
+	int cpu, i;
 
 	pgdat_init_internals(pgdat);
 
@@ -1554,10 +1554,17 @@ void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
 	pgdat->node_start_pfn = 0;
 	pgdat->node_present_pages = 0;
 
-	for_each_online_cpu(cpu) {
-		struct per_cpu_nodestat *p;
+	/*
+	 * Hot-unplug can leave per-cpu vmstat deltas unfolded (folders skip
+	 * offline nodes) - reconcile this at online. Foreign access to counters
+	 * is safe: the node is not online yet and we hold the hotplug lock.
+	 */
+	for_each_possible_cpu(cpu) {
+		struct per_cpu_nodestat *p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
 
-		p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
+		for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++)
+			if (p->vm_node_stat_diff[i])
+				node_page_state_add(p->vm_node_stat_diff[i], pgdat, i);
 		memset(p, 0, sizeof(*p));
 	}
 
-- 
2.53.0-Meta


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

* Re: [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online
  2026-06-27 20:22 [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online Gregory Price
@ 2026-06-27 23:10 ` Andrew Morton
  2026-06-28  0:31   ` Gregory Price
  2026-06-30 20:57   ` Gregory Price
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2026-06-27 23:10 UTC (permalink / raw)
  To: Gregory Price
  Cc: linux-mm, linux-kernel, kernel-team, rppt, vbabka, mgorman,
	hannes, stable

On Sat, 27 Jun 2026 16:22:43 -0400 Gregory Price <gourry@gourry.net> wrote:

> A per-node vmstat counter is pgdat->vm_stat[] plus per-cpu deltas.
> A balanced counter can sit split as global=+N / per-cpu=-N.
> 
> The folds reconciling the split only walk online nodes, so when
> try_offline_node() marks a node offline the per-cpu deltas are stranded.
> 
> A subsequent online resets the per-cpu area but not pgdat->vm_stat[],
> orphaning the +N permanently.  All NR_VM_NODE_STAT_ITEMS are affected.

Geeze, simple mistake, been there ten years...

> The existing code zeroes the per-cpu counters and causes a permanent
> skew. Fold the stranded deltas instead, before the node rejoins the
> online set. The node is not online yet and the hotplug lock is held,
> so the remote access to per-cpu values is safe.

Oh.  Shouldn't we be doing this during offlining?

> Discovered when node compaction hung for a nearly empty node, as the
> math to determine throttling broke.  Reproduced by repeated memory
> hotplug/unplug cycles on a node under pressure: NR_ISOLATED_ANON
> ratchets up and never returns to zero.
> 
> ...
>
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -1536,7 +1536,7 @@ void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
>  {
>  	int nid = pgdat->node_id;
>  	enum zone_type z;
> -	int cpu;
> +	int cpu, i;
>  
>  	pgdat_init_internals(pgdat);
>  
> @@ -1554,10 +1554,17 @@ void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
>  	pgdat->node_start_pfn = 0;
>  	pgdat->node_present_pages = 0;
>  
> -	for_each_online_cpu(cpu) {
> -		struct per_cpu_nodestat *p;
> +	/*
> +	 * Hot-unplug can leave per-cpu vmstat deltas unfolded (folders skip
> +	 * offline nodes) - reconcile this at online. Foreign access to counters
> +	 * is safe: the node is not online yet and we hold the hotplug lock.
> +	 */
> +	for_each_possible_cpu(cpu) {

That's a lot of CPUs

> +		struct per_cpu_nodestat *p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
>  
> -		p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> +		for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++)

and that's a lot of items.

I guess the overall loop count won't be large enough to cause issues,
but it's large!

Perhaps there's some simple test we can do on the per_cpu_nodestat to
avoid the inner loop?  Perhaps might need to add a field for this?

btw, "for(int i..." is allowed nowadays.  It'll make this code nicer, IMO.

And... Sashiko seems to have found a pre-existing issue:
	https://sashiko.dev/#/patchset/20260627202243.758289-1-gourry@gourry.net

> +			if (p->vm_node_stat_diff[i])
> +				node_page_state_add(p->vm_node_stat_diff[i], pgdat, i);
>  		memset(p, 0, sizeof(*p));
>  	}


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

* Re: [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online
  2026-06-27 23:10 ` Andrew Morton
@ 2026-06-28  0:31   ` Gregory Price
  2026-06-30 20:57   ` Gregory Price
  1 sibling, 0 replies; 6+ messages in thread
From: Gregory Price @ 2026-06-28  0:31 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, rppt, vbabka, mgorman,
	hannes, stable

On Sat, Jun 27, 2026 at 04:10:07PM -0700, Andrew Morton wrote:
> 
> > The existing code zeroes the per-cpu counters and causes a permanent
> > skew. Fold the stranded deltas instead, before the node rejoins the
> > online set. The node is not online yet and the hotplug lock is held,
> > so the remote access to per-cpu values is safe.
> 
> Oh.  Shouldn't we be doing this during offlining?
>

I tried this first.  I was unable to convince myself there was a 
safe way to accomplish this.

1) sashiko pointed out we can't schedule_on_each_cpu while holding
   the hotplug lock because we'll re-take cpus_read_lock and cause
   a deadlock condition with cpu-hotplug.

2) I'm not sure we can do it after the hotplug lock as been dropped,
   at least not safely.  At the very least another hot-plug re-adding
   the node could start.  That just seemed like a bad path.

3) foreign cpu access to the per-cpu values are not atomic with
   respect to in-flight folds on the target cpu.  this_cpu_xchg
   and this_cpu_add are (i believe) only atomic wrt the cpu itself
   (can't be interrupted mid-exchange).

doing it before node_offline() has problems (in-flight folds),
doing it after node_offline() still *technically* carries the same
in-flight fold risk - just narrower (fold has to have started already).

I couldn't convince myself there wasn't still a race, so here we are.

> > +	for_each_possible_cpu(cpu) {
> 
> That's a lot of CPUs
> 

Unfortunately - cpus may have gone offline while the node was offline,
so we legitimately have to visit every *possible* cpu :[

> > +		struct per_cpu_nodestat *p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> >  
> > -		p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> > +		for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++)
> 
> and that's a lot of items.

I am aware :[.  I suppose we could vectorize the collection here on some
archs, but I try to avoid being clever where I can.

> 
> I guess the overall loop count won't be large enough to cause issues,
> but it's large!
> 
> Perhaps there's some simple test we can do on the per_cpu_nodestat to
> avoid the inner loop?  Perhaps might need to add a field for this?

Hadn't considered this, but maybe.  Will take a look.

> 
> btw, "for(int i..." is allowed nowadays.  It'll make this code nicer, IMO.
> 

aye aye o7

> And... Sashiko seems to have found a pre-existing issue:
> 	https://sashiko.dev/#/patchset/20260627202243.758289-1-gourry@gourry.net
> 

Will take a look, thanks!

~Gregory

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

* Re: [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online
  2026-06-27 23:10 ` Andrew Morton
  2026-06-28  0:31   ` Gregory Price
@ 2026-06-30 20:57   ` Gregory Price
  2026-06-30 22:55     ` Andrew Morton
  1 sibling, 1 reply; 6+ messages in thread
From: Gregory Price @ 2026-06-30 20:57 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, rppt, vbabka, mgorman,
	hannes, stable

On Sat, Jun 27, 2026 at 04:10:07PM -0700, Andrew Morton wrote:
> On Sat, 27 Jun 2026 16:22:43 -0400 Gregory Price <gourry@gourry.net> wrote:
> 
> > +		struct per_cpu_nodestat *p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> >  
> > -		p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> > +		for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++)
> 
> and that's a lot of items.
> 
> I guess the overall loop count won't be large enough to cause issues,
> but it's large!
> 
> Perhaps there's some simple test we can do on the per_cpu_nodestat to
> avoid the inner loop?  Perhaps might need to add a field for this?
>

I took a look, but that would involve adding another per-cpu field and
then making sure all the races on that field are respected as well.

Not sure it's worth it for such an extremely rare event.

I can try to get clever on the folding logic if you'd like, let me know.

> btw, "for(int i..." is allowed nowadays.  It'll make this code nicer, IMO.
> 

Otherwise i can send you a respin for this.

> And... Sashiko seems to have found a pre-existing issue:
> 	https://sashiko.dev/#/patchset/20260627202243.758289-1-gourry@gourry.net
> 

Incoming patch for this shortly.  Pretty trivial.

~Gregory

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

* Re: [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online
  2026-06-30 20:57   ` Gregory Price
@ 2026-06-30 22:55     ` Andrew Morton
  2026-06-30 23:27       ` Gregory Price
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2026-06-30 22:55 UTC (permalink / raw)
  To: Gregory Price
  Cc: linux-mm, linux-kernel, kernel-team, rppt, vbabka, mgorman,
	hannes, stable

On Tue, 30 Jun 2026 16:57:43 -0400 Gregory Price <gourry@gourry.net> wrote:

> On Sat, Jun 27, 2026 at 04:10:07PM -0700, Andrew Morton wrote:
> > On Sat, 27 Jun 2026 16:22:43 -0400 Gregory Price <gourry@gourry.net> wrote:
> > 
> > > +		struct per_cpu_nodestat *p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> > >  
> > > -		p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> > > +		for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++)
> > 
> > and that's a lot of items.
> > 
> > I guess the overall loop count won't be large enough to cause issues,
> > but it's large!
> > 
> > Perhaps there's some simple test we can do on the per_cpu_nodestat to
> > avoid the inner loop?  Perhaps might need to add a field for this?
> >
> 
> I took a look, but that would involve adding another per-cpu field and
> then making sure all the races on that field are respected as well.
> 
> Not sure it's worth it for such an extremely rare event.
> 
> I can try to get clever on the folding logic if you'd like, let me know.
> 
> > btw, "for(int i..." is allowed nowadays.  It'll make this code nicer, IMO.
> > 
> 
> Otherwise i can send you a respin for this.

Is OK, we could make this change in a million other places.

> > And... Sashiko seems to have found a pre-existing issue:
> > 	https://sashiko.dev/#/patchset/20260627202243.758289-1-gourry@gourry.net
> > 
> 
> Incoming patch for this shortly.  Pretty trivial.

Cool, what was the Subject?


I'll queue this patch in mm-hotfixes for some testing while we await
further review (please).


From: Gregory Price <gourry@gourry.net>
Subject: mm/vmstat: fold stranded per-cpu node stats when a node comes online
Date: Sat, 27 Jun 2026 16:22:43 -0400

A per-node vmstat counter is pgdat->vm_stat[] plus per-cpu deltas.  A
balanced counter can sit split as global=+N / per-cpu=-N.

The folds reconciling the split only walk online nodes, so when
try_offline_node() marks a node offline the per-cpu deltas are stranded.

A subsequent online resets the per-cpu area but not pgdat->vm_stat[],
orphaning the +N permanently.  All NR_VM_NODE_STAT_ITEMS are affected.

The existing code zeroes the per-cpu counters and causes a permanent skew.
Fold the stranded deltas instead, before the node rejoins the online set.
The node is not online yet and the hotplug lock is held, so the remote
access to per-cpu values is safe.

Discovered when node compaction hung for a nearly empty node, as the math
to determine throttling broke.  Reproduced by repeated memory
hotplug/unplug cycles on a node under pressure: NR_ISOLATED_ANON ratchets
up and never returns to zero.

Link: https://lore.kernel.org/20260627202243.758289-1-gourry@gourry.net
Fixes: 75ef71840539 ("mm, vmstat: add infrastructure for per-node vmstats")
Signed-off-by: Gregory Price <gourry@gourry.net>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/mm_init.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

--- a/mm/mm_init.c~mm-vmstat-fold-stranded-per-cpu-node-stats-when-a-node-comes-online
+++ a/mm/mm_init.c
@@ -1540,7 +1540,7 @@ void __ref free_area_init_core_hotplug(s
 {
 	int nid = pgdat->node_id;
 	enum zone_type z;
-	int cpu;
+	int cpu, i;
 
 	pgdat_init_internals(pgdat);
 
@@ -1558,10 +1558,17 @@ void __ref free_area_init_core_hotplug(s
 	pgdat->node_start_pfn = 0;
 	pgdat->node_present_pages = 0;
 
-	for_each_online_cpu(cpu) {
-		struct per_cpu_nodestat *p;
+	/*
+	 * Hot-unplug can leave per-cpu vmstat deltas unfolded (folders skip
+	 * offline nodes) - reconcile this at online. Foreign access to counters
+	 * is safe: the node is not online yet and we hold the hotplug lock.
+	 */
+	for_each_possible_cpu(cpu) {
+		struct per_cpu_nodestat *p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
 
-		p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
+		for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++)
+			if (p->vm_node_stat_diff[i])
+				node_page_state_add(p->vm_node_stat_diff[i], pgdat, i);
 		memset(p, 0, sizeof(*p));
 	}
 
_


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

* Re: [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online
  2026-06-30 22:55     ` Andrew Morton
@ 2026-06-30 23:27       ` Gregory Price
  0 siblings, 0 replies; 6+ messages in thread
From: Gregory Price @ 2026-06-30 23:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-kernel, kernel-team, rppt, vbabka, mgorman,
	hannes, stable

On Tue, Jun 30, 2026 at 03:55:17PM -0700, Andrew Morton wrote:
> On Tue, 30 Jun 2026 16:57:43 -0400 Gregory Price <gourry@gourry.net> wrote:
> 
> > On Sat, Jun 27, 2026 at 04:10:07PM -0700, Andrew Morton wrote:
> > > On Sat, 27 Jun 2026 16:22:43 -0400 Gregory Price <gourry@gourry.net> wrote:
> > > 
> > > > +		struct per_cpu_nodestat *p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> > > >  
> > > > -		p = per_cpu_ptr(pgdat->per_cpu_nodestats, cpu);
> > > > +		for (i = 0; i < NR_VM_NODE_STAT_ITEMS; i++)
> > > 
> > > and that's a lot of items.
> > > 
> > > I guess the overall loop count won't be large enough to cause issues,
> > > but it's large!
> > > 
> > > Perhaps there's some simple test we can do on the per_cpu_nodestat to
> > > avoid the inner loop?  Perhaps might need to add a field for this?
> > >
> > 
> > I took a look, but that would involve adding another per-cpu field and
> > then making sure all the races on that field are respected as well.
> > 
> > Not sure it's worth it for such an extremely rare event.
> > 
> > I can try to get clever on the folding logic if you'd like, let me know.
> > 
> > > btw, "for(int i..." is allowed nowadays.  It'll make this code nicer, IMO.
> > > 
> > 
> > Otherwise i can send you a respin for this.
> 
> Is OK, we could make this change in a million other places.
> 
> > > And... Sashiko seems to have found a pre-existing issue:
> > > 	https://sashiko.dev/#/patchset/20260627202243.758289-1-gourry@gourry.net
> > > 
> > 
> > Incoming patch for this shortly.  Pretty trivial.
> 
> Cool, what was the Subject?
>

[PATCH] mm/mm_init: handle alloc_percpu failure in free_area_init_core_hotplug

https://lore.kernel.org/linux-mm/20260630214039.2263562-1-gourry@gourry.net/T/#u

I didn't bother with Cc:stable because it's also 10 years old and
doesn't seem likely to actually get hit, but if you think it should be
stable let me know.

> 
> I'll queue this patch in mm-hotfixes for some testing while we await
> further review (please).
>

Thank you!

~Gregory

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

end of thread, other threads:[~2026-06-30 23:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-27 20:22 [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online Gregory Price
2026-06-27 23:10 ` Andrew Morton
2026-06-28  0:31   ` Gregory Price
2026-06-30 20:57   ` Gregory Price
2026-06-30 22:55     ` Andrew Morton
2026-06-30 23:27       ` Gregory Price

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