Re: [PATCH v2] mm/vmstat: fold stranded per-cpu node stats when a node comes online

[Gregory Price <gourry@gourry.net>]

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