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

Oscar Salvador
2025-04-02T16:57:54Z

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