Re: [PATCH v6 03/10] mm,memory_hotplug: Implement numa node notifier

David Hildenbrand
2025-06-10T08:10:33Z

On 09.06.25 11:21, Oscar Salvador wrote:
> There are at least six consumers of hotplug_memory_notifier that what they
> really are interested in is whether any numa node changed its state, e.g: going
> from having memory to not having memory and vice versa.
>
> Implement a specific notifier for numa nodes when their state gets changed,
> which will later be used by those consumers that are only interested
> in numa node state changes.
>
> Add documentation as well.
>
> Signed-off-by: Oscar Salvador <osalvador@suse.de>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> Documentation/core-api/memory-hotplug.rst | 66 +++++++++
> drivers/base/node.c | 21 +++
> include/linux/node.h | 40 ++++++
> mm/memory_hotplug.c | 155 ++++++++++------------
> 4 files changed, 200 insertions(+), 82 deletions(-)
>
> diff --git a/Documentation/core-api/memory-hotplug.rst b/Documentation/core-api/memory-hotplug.rst
> index d1b8eb9add8a..b19c3be7437d 100644
> --- a/Documentation/core-api/memory-hotplug.rst
> +++ b/Documentation/core-api/memory-hotplug.rst
> @@ -9,6 +9,9 @@ Memory hotplug event notifier
>
> Hotplugging events are sent to a notification queue.
>
> +Memory notifier
> +----------------
> +
> There are six types of notification defined in ``include/linux/memory.h``:
>
> MEM_GOING_ONLINE
> @@ -80,6 +83,69 @@ further processing of the notification queue.
>
> NOTIFY_STOP stops further processing of the notification queue.
>
> +Numa node notifier
> +------------------
> +
> +There are six types of notification defined in ``include/linux/node.h``:
> +
> +NODE_ADDING_FIRST_MEMORY
> + Generated before memory becomes available to this node for the first time.
> +
> +NODE_CANCEL_ADDING_FIRST_MEMORY
> + Generated if NODE_ADDING_FIRST_MEMORY fails.
> +
> +NODE_ADDED_FIRST_MEMORY
> + Generated when memory has become available fo this node for the first time.
> +
> +NODE_REMOVING_LAST_MEMORY
> + Generated when the last memory available to this node is about to be offlined.
> +
> +NODE_CANCEL_REMOVING_LAST_MEMORY
> + Generated when NODE_CANCEL_REMOVING_LAST_MEMORY fails.
> +
> +NODE_REMOVED_LAST_MEMORY
> + Generated when the last memory available to this node has been offlined.
> +
> +A callback routine can be registered by calling::
> +
> + hotplug_node_notifier(callback_func, priority)
> +
> +Callback functions with higher values of priority are called before callback
> +functions with lower values.
> +
> +A callback function must have the following prototype::
> +
> + int callback_func(
> +
> + struct notifier_block *self, unsigned long action, void *arg);
> +
> +The first argument of the callback function (self) is a pointer to the block
> +of the notifier chain that points to the callback function itself.
> +The second argument (action) is one of the event types described above.
> +The third argument (arg) passes a pointer of struct node_notify::
> +
> + struct node_notify {
> + int nid;
> + }
> +
> +- nid is the node we are adding or removing memory to.
> +
> + If nid >= 0, callback should create/discard structures for the
> + node if necessary.

Likely that should be removed?

It' probably worth mentioning that one might get notified about
NODE_CANCEL_ADDING_FIRST_MEMORY even though never notified for
NODE_ADDING_FIRST_MEMORY. (same for removing)

I recall this can happen if one of the NODE_ADDING_FIRST_MEMORY
notifiers fails.

(same applies to MEM_CANCEL_*)

Consequently, we might simplify the cancel_mem_notifier_on_err etc
stuff, simply unconditionally calling the cancel counterparts.

> +
> +The callback routine shall return one of the values
> +NOTIFY_DONE, NOTIFY_OK, NOTIFY_BAD, NOTIFY_STOP
> +defined in ``include/linux/notifier.h``
> +
> +NOTIFY_DONE and NOTIFY_OK have no effect on the further processing.
> +
> +NOTIFY_BAD is used as response to the NODE_ADDING_FIRST_MEMORY,
> +NODE_REMOVING_LAST_MEMORY, NODE_ADDED_FIRST_MEMORY or
> +NODE_REMOVED_LAST_MEMORY action to cancel hotplugging.
> +It stops further processing of the notification queue.
> +
> +NOTIFY_STOP stops further processing of the notification queue.

Should we docunment that failing NODE_ADDED_FIRST_MEMORY /
NODE_REMOVED_FIRST_MEMORY is very bad?

> +
> Locking Internals
> =================
>
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 25ab9ec14eb8..c5b0859d846d 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -111,6 +111,27 @@ static const struct attribute_group *node_access_node_groups[] = {
> NULL,
> };
>
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +static BLOCKING_NOTIFIER_HEAD(node_chain);
> +
> +int register_node_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&node_chain, nb);
> +}
> +EXPORT_SYMBOL(register_node_notifier);
> +
> +void unregister_node_notifier(struct notifier_block *nb)
> +{
> + blocking_notifier_chain_unregister(&node_chain, nb);
> +}
> +EXPORT_SYMBOL(unregister_node_notifier);
> +
> +int node_notify(unsigned long val, void *v)
> +{
> + return blocking_notifier_call_chain(&node_chain, val, v);
> +}
> +#endif
> +
> static void node_remove_accesses(struct node *node)
> {
> struct node_access_nodes *c, *cnext;
> diff --git a/include/linux/node.h b/include/linux/node.h
> index 2b7517892230..d7aa2636d948 100644
> --- a/include/linux/node.h
> +++ b/include/linux/node.h
> @@ -123,6 +123,46 @@ static inline void register_memory_blocks_under_node(int nid, unsigned long star
> #endif
>
> extern void unregister_node(struct node *node);
> +
> +struct node_notify {
> + int nid;
> +};
> +
> +#define NODE_ADDING_FIRST_MEMORY (1<<0)
> +#define NODE_ADDED_FIRST_MEMORY (1<<1)
> +#define NODE_CANCEL_ADDING_FIRST_MEMORY (1<<2)
> +#define NODE_REMOVING_LAST_MEMORY (1<<3)
> +#define NODE_REMOVED_LAST_MEMORY (1<<4)
> +#define NODE_CANCEL_REMOVING_LAST_MEMORY (1<<5)
> +
> +#if defined(CONFIG_MEMORY_HOTPLUG) && defined(CONFIG_NUMA)
> +extern int register_node_notifier(struct notifier_block *nb);
> +extern void unregister_node_notifier(struct notifier_block *nb);
> +extern int node_notify(unsigned long val, void *v);
> +
> +#define hotplug_node_notifier(fn, pri) ({ \
> + static __meminitdata struct notifier_block fn##_node_nb =\
> + { .notifier_call = fn, .priority = pri };\
> + register_node_notifier(&fn##_node_nb); \
> +})
> +#else
> +static inline int register_node_notifier(struct notifier_block *nb)
> +{
> + return 0;
> +}
> +static inline void unregister_node_notifier(struct notifier_block *nb)
> +{
> +}
> +static inline int node_notify(unsigned long val, void *v)
> +{
> + return 0;
> +}
> +static inline int hotplug_node_notifier(notifier_fn_t fn, int pri)
> +{
> + return 0;
> +}
> +#endif
> +
> #ifdef CONFIG_NUMA
> extern void node_dev_init(void);
> /* Core of the node registration - only memory hotplug should use this */
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 94ae0ca37021..0550f3061fc4 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -35,6 +35,7 @@
> #include <linux/compaction.h>
> #include <linux/rmap.h>
> #include <linux/module.h>
> +#include <linux/node.h>
>
> #include <asm/tlbflush.h>
>
> @@ -699,24 +700,6 @@ static void online_pages_range(unsigned long start_pfn, unsigned long nr_pages)
> online_mem_sections(start_pfn, end_pfn);
> }
>
> -/* check which state of node_states will be changed when online memory */
> -static void node_states_check_changes_online(unsigned long nr_pages,
> - struct zone *zone, struct memory_notify *arg)
> -{
> - int nid = zone_to_nid(zone);
> -
> - arg->status_change_nid = NUMA_NO_NODE;
> -
> - if (!node_state(nid, N_MEMORY))
> - arg->status_change_nid = nid;
> -}
> -
> -static void node_states_set_node(int node, struct memory_notify *arg)
> -{
> - if (arg->status_change_nid >= 0)
> - node_set_state(node, N_MEMORY);
> -}
> -
> static void __meminit resize_zone_range(struct zone *zone, unsigned long start_pfn,
> unsigned long nr_pages)
> {
> @@ -1171,7 +1154,9 @@ int online_pages(unsigned long pfn, unsigned long nr_pages,
> int need_zonelists_rebuild = 0;
> const int nid = zone_to_nid(zone);
> int ret;
> - struct memory_notify arg;
> + struct memory_notify mem_arg;
> + struct node_notify node_arg;
> + bool cancel_mem_notifier_on_err = false, cancel_node_notifier_on_err = false;

Prefer reverse xmas tree ... :)

[...]

> -
> static int count_system_ram_pages_cb(unsigned long start_pfn,
> unsigned long nr_pages, void *data)
> {
> @@ -1937,13 +1899,17 @@ static int count_system_ram_pages_cb(unsigned long start_pfn,
> int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
> struct zone *zone, struct memory_group *group)
> {
> - const unsigned long end_pfn = start_pfn + nr_pages;
> - unsigned long pfn, managed_pages, system_ram_pages = 0;
> - const int node = zone_to_nid(zone);
> - unsigned long flags;
> - struct memory_notify arg;
> - char *reason;
> int ret;
> + char *reason;
> + enum zone_type zt;
> + unsigned long flags;
> + struct memory_notify mem_arg;
> + struct node_notify node_arg;
> + const int node = zone_to_nid(zone);
> + struct pglist_data *pgdat = zone->zone_pgdat;
> + const unsigned long end_pfn = start_pfn + nr_pages;
> + unsigned long pfn, managed_pages, system_ram_pages = 0, present_pages = 0;
> + bool cancel_mem_notifier_on_err = false, cancel_node_notifier_on_err = false;

You'r now reversing the reversed christmas tree :)

>
> /*
> * {on,off}lining is constrained to full memory sections (or more
> @@ -2000,11 +1966,30 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
> goto failed_removal_pcplists_disabled;
> }
>
> - arg.start_pfn = start_pfn;
> - arg.nr_pages = nr_pages;
> - node_states_check_changes_offline(nr_pages, zone, &arg);
> + /*
> + * Here we count the possible pages within the range [0..ZONE_MOVABLE].
> + * If after having accounted all the pages, we see that the nr_pages to
> + * be offlined is greater or equal to the accounted pages, we know that the
> + * node will become empty, and so, we will clear N_MEMORY for it.
> + */
> + node_arg.nid = NUMA_NO_NODE;
> + for (zt = 0; zt <= ZONE_MOVABLE; zt++)
> + present_pages += pgdat->node_zones[zt].present_pages;

Why can't we look at node_present_pages?

--
Cheers,

David / dhildenb