Re: [PATCH 1/2] mm,memory_hotplug: Implement numa node notifier

[Oscar Salvador]

On Wed, Apr 02, 2025 at 06:03:04PM +0200, Vlastimil Babka wrote:
> On 4/1/25 11:27, Oscar Salvador wrote:
> > -	arg.start_pfn = pfn;
> > -	arg.nr_pages = nr_pages;
> > -	node_states_check_changes_online(nr_pages, zone, &arg);
> > +	mem_arg.start_pfn = pfn;
> > +	mem_arg.nr_pages = nr_pages;
> > +	node_states_check_changes_online(nr_pages, zone, &node_arg);
> >  
> > -	ret = memory_notify(MEM_GOING_ONLINE, &arg);
> > +	if (node_arg.status_change_nid >= 0) {
> 
> Hmm, don't we need to add "|| node_arg.status_change_nid_normal >= 0"? Or we
> fail to notify addition of normal memory to a node that already has !normal
> memory?

Ah, I think you are right, nice catch.
Yes, if we add normal (<= ZONE_NORMAL) memory to a node that only has ZONE_MOVABLE,
status_change_nid_normal will change, but status_change_nid will not.

So yes, we need a "|| node_arg.status_change_nid_normal >= 0".

> > -	memory_notify(MEM_CANCEL_ONLINE, &arg);
> > +	if (cancel_node_notifier_on_err)
> > +		node_notify(NODE_CANCEL_MEM_AWARE, &node_arg);
> > +	if (cancel_mem_notifier_on_err)
> > +		memory_notify(MEM_CANCEL_ONLINE, &mem_arg);
> 
> Switch the order of those just for symmetry? :)

Will do.

> > -	arg.start_pfn = start_pfn;
> > -	arg.nr_pages = nr_pages;
> > -	node_states_check_changes_offline(nr_pages, zone, &arg);
> > +	mem_arg.start_pfn = start_pfn;
> > +	mem_arg.nr_pages = nr_pages;
> > +	node_states_check_changes_offline(nr_pages, zone, &node_arg);
> > +
> > +	if (node_arg.status_change_nid >= 0) {
> 
> Ditto.

Yap.

> >  failed_removal_isolated:
> >  	/* pushback to free area */
> >  	undo_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE);
> > -	memory_notify(MEM_CANCEL_OFFLINE, &arg);
> > +	if (cancel_node_notifier_on_err)
> > +		node_notify(NODE_CANCEL_MEMORYLESS, &node_arg);
> > +	if (cancel_mem_notifier_on_err)
> > +		memory_notify(MEM_CANCEL_OFFLINE, &mem_arg);
> 
> Ditto.

Copy that.

Thanks Vlastimil!

-- 
Oscar Salvador
SUSE Labs