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

[Gregory Price <gourry@gourry.net>]

On Wed, Jul 01, 2026 at 10:35:41AM +0200, David Hildenbrand (Arm) wrote:
> 
> >  	 * The node we allocated has no zone fallback lists. For avoiding
> > diff --git a/mm/mm_init.c b/mm/mm_init.c
> > index 306ea5c13f54..37fd64ce144d 100644
> > --- a/mm/mm_init.c
> > +++ b/mm/mm_init.c
> > @@ -1536,7 +1536,7 @@ void __init set_pageblock_order(void)
> >   * NOTE: this function is only called during memory hotplug
> >   */
> >  #ifdef CONFIG_MEMORY_HOTPLUG
> > -void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
> > +int __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
> >  {
> >  	int nid = pgdat->node_id;
> >  	enum zone_type z;
> > @@ -1544,8 +1544,14 @@ void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
> >  
> >  	pgdat_init_internals(pgdat);
> >  
> > -	if (pgdat->per_cpu_nodestats == &boot_nodestats)
> > -		pgdat->per_cpu_nodestats = alloc_percpu(struct per_cpu_nodestat);
> > +	if (pgdat->per_cpu_nodestats == &boot_nodestats) {
> > +		struct per_cpu_nodestat __percpu *p;
> > +
> > +		p = alloc_percpu(struct per_cpu_nodestat);
> > +		if (!p)
> > +			return -ENOMEM;
> > +		pgdat->per_cpu_nodestats = p;
> 
> Is there a need for the temporary variable?
> 

at start:
    pgdat->per_cpu_nodestats = &boot_nodestats 

So need a tmp to do the swap/revert/etc

> Also how to handle cleanup on error? Or why can we skip cleanup? (what happens
> if we get another call to __try_online_node()  later?)
> 

-ENOMEM ->
    hotadd_init_pgdat -> NULL
        __try_online_node -> -ENOMEM
	    pr_err("Cannot online node %d due to NULL pgdat\n", nid);
	    try_online_node -> -ENOMEM
	        __add_memory_resource  ->  error_memblock_remove:

Basically we're left exactly where we were before we made the attempt.

Another call should work just fine after a bunch of dmesg spew.

If this happens during boot via add_memory then something else is
horribly horribly wrong and we'll at least get some debug info instead
of null deref.

~Gregory