Re: [PATCH v2] mm/memory_hotplug: maintain N_NORMAL_MEMORY during hotplug

[Hao Li]

On Mon, Mar 30, 2026 at 04:20:23PM +0200, David Hildenbrand (Arm) wrote:
> On 3/30/26 05:57, Hao Li wrote:
> > N_NORMAL_MEMORY is initialized from zone population at boot, but memory
> > hotplug currently only updates N_MEMORY. As a result, a node that gains
> > normal memory via hotplug can remain invisible to users iterating over
> > N_NORMAL_MEMORY, while a node that loses its last normal memory can stay
> > incorrectly marked as such.
> > 
> > Restore N_NORMAL_MEMORY maintenance directly in online_pages() and
> > offline_pages(). Set the bit when a node that currently lacks normal
> > memory onlines pages into a zone <= ZONE_NORMAL, and clear it when
> > offlining removes the last present pages from zones <= ZONE_NORMAL.
> > 
> > This restores the intended semantics without bringing back the old
> > status_change_nid_normal notifier plumbing which was removed in
> > 8d2882a8edb8.
> > 
> > Current users that benefit include list_lru, zswap, nfsd filecache,
> > hugetlb_cgroup, and has_normal_memory sysfs reporting.
> > 
> > Fixes: 8d2882a8edb8 ("mm,memory_hotplug: remove status_change_nid_normal and update documentation")
> 
> Agreed on CCing stable.
> 
> > Signed-off-by: Hao Li <hao.li@xxxxxxxxx>
> > ---
> > Changes: simplify the code. (Thanks Joshua and David)
> > ---
> >  mm/memory_hotplug.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index bc805029da51..05a47953ef21 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -1209,6 +1209,13 @@ int online_pages(unsigned long pfn, unsigned long nr_pages,
> >  
> >  	if (node_arg.nid >= 0)
> >  		node_set_state(nid, N_MEMORY);
> > +	/*
> > +	 * Check whether we are adding normal memory to the node for the first
> > +	 * time.
> > +	 */
> > +	if (!node_state(nid, N_NORMAL_MEMORY) && zone_idx(zone) <= ZONE_NORMAL)
> > +		node_set_state(nid, N_NORMAL_MEMORY);
> > +
> >  	if (need_zonelists_rebuild)
> >  		build_all_zonelists(NULL);
> >  
> > @@ -1908,6 +1915,8 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
> >  	unsigned long flags;
> >  	char *reason;
> >  	int ret;
> > +	unsigned long normal_pages = 0;
> > +	enum zone_type zt;
> >  
> >  	/*
> >  	 * {on,off}lining is constrained to full memory sections (or more
> > @@ -2055,6 +2064,17 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
> >  	/* reinitialise watermarks and update pcp limits */
> >  	init_per_zone_wmark_min();
> >  
> > +	/*
> > +	 * Check whether this operation removes the last normal memory from
> > +	 * the node. We do this before clearing N_MEMORY to avoid the possible
> > +	 * transient "!N_MEMORY && N_NORMAL_MEMORY" state.
> > +	 */
> > +	if (zone_idx(zone) <= ZONE_NORMAL) {
> > +		for (zt = 0; zt <= ZONE_NORMAL; zt++)
> > +			normal_pages += pgdat->node_zones[zt].present_pages;
> > +		if (!normal_pages)
> > +			node_clear_state(node, N_NORMAL_MEMORY);
> > +	}
> 
> Was wondering whether we could just do an early exit, like:
> 
> 
> for (zt = 0; zt <= ZONE_NORMAL; zt++) {
> 	if (pgdat->node_zones[zt].present_pages)
> 		break;
> }
> if (zt > ZONE_NORMAL)
> 	node_clear_state(node, N_NORMAL_MEMORY);
> 
> But ends up being harder to digest (incl. all variants I tried :) ), so

Thanks for exploring this.
Yeah, this looks like one of those cases where we have to make a bit of a
tradeoff for readability :)

> 
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Thanks for the Ack!

-- 
Thanks,
Hao