[PATCH 0/2] drivers/base/node: fold node register and unregister functions

[PATCH 0/2] drivers/base/node: fold node register and unregister functions

[Donet Tom]

This change came from the discussion in [1]. It consists of two patches:

The first patch merges register_one_node() and register_node(), leaving a
single register_node() function.

The second patch merges unregister_one_node() and unregister_node(), leaving
a single unregister_node() function.

There is no functional change in these patches.

[1] https://lore.kernel.org/all/5512b1b6-31f8-4322-8a5f-add8d1e9b22f@redhat.com/

Donet Tom (2):
  drivers/base/node: merge register_one_node() and register_node() to a
    single function.
  drivers/base/node: merge unregister_one_node() and unregister_node()
    to a single function.

 arch/powerpc/platforms/pseries/pci_dlpar.c |  2 +-
 arch/x86/mm/numa.c                         |  4 +-
 drivers/base/node.c                        | 89 +++++++++-------------
 include/linux/node.h                       | 10 +--
 mm/memory_hotplug.c                        |  8 +-
 mm/mm_init.c                               |  2 +-
 6 files changed, 49 insertions(+), 66 deletions(-)

-- 
2.51.0

[PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.

[Donet Tom]

register_one_node() and register_node() are small functions.
This patch merges them into a single function named register_node()
to improve code readability.

No functional changes are introduced.

Signed-off-by: Donet Tom <donettom@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/pci_dlpar.c |  2 +-
 arch/x86/mm/numa.c                         |  4 +-
 drivers/base/node.c                        | 52 +++++++++-------------
 include/linux/node.h                       |  4 +-
 mm/memory_hotplug.c                        |  4 +-
 mm/mm_init.c                               |  2 +-
 6 files changed, 28 insertions(+), 40 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
index aeb8633a3d00..8c77ec7980de 100644
--- a/arch/powerpc/platforms/pseries/pci_dlpar.c
+++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
@@ -29,7 +29,7 @@ struct pci_controller *init_phb_dynamic(struct device_node *dn)
 	nid = of_node_to_nid(dn);
 	if (likely((nid) >= 0)) {
 		if (!node_online(nid)) {
-			if (register_one_node(nid)) {
+			if (register_node(nid)) {
 				pr_err("PCI: Failed to register node %d\n", nid);
 			} else {
 				update_numa_distance(dn);
diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index c24890c40138..7a97327140df 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -262,7 +262,7 @@ void __init init_gi_nodes(void)
 	 * bringup_nonboot_cpus
 	 *  cpu_up
 	 *   __try_online_node
-	 *    register_one_node
+	 *    register_node
 	 * because node_subsys is not initialized yet.
 	 * TODO remove dependency on node_online
 	 */
@@ -303,7 +303,7 @@ void __init init_cpu_to_node(void)
 		 * bringup_nonboot_cpus
 		 *  cpu_up
 		 *   __try_online_node
-		 *    register_one_node
+		 *    register_node
 		 * because node_subsys is not initialized yet.
 		 * TODO remove dependency on node_online
 		 */
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 6b6e55a98b79..eab620e29c78 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -638,33 +638,6 @@ static void node_device_release(struct device *dev)
 	kfree(to_node(dev));
 }
 
-/*
- * register_node - Setup a sysfs device for a node.
- * @num - Node number to use when creating the device.
- *
- * Initialize and register the node device.
- */
-static int register_node(struct node *node, int num)
-{
-	int error;
-
-	node->dev.id = num;
-	node->dev.bus = &node_subsys;
-	node->dev.release = node_device_release;
-	node->dev.groups = node_dev_groups;
-	error = device_register(&node->dev);
-
-	if (error) {
-		put_device(&node->dev);
-	} else {
-		hugetlb_register_node(node);
-		compaction_register_node(node);
-		reclaim_register_node(node);
-	}
-
-	return error;
-}
-
 /**
  * unregister_node - unregister a node device
  * @node: node going away
@@ -869,7 +842,13 @@ void register_memory_blocks_under_node_hotplug(int nid, unsigned long start_pfn,
 }
 #endif /* CONFIG_MEMORY_HOTPLUG */
 
-int register_one_node(int nid)
+/*
+ * register_node - Setup a sysfs device for a node.
+ * @nid - Node number to use when creating the device.
+ *
+ * Initialize and register the node device.
+ */
+int register_node(int nid)
 {
 	int error;
 	int cpu;
@@ -880,14 +859,23 @@ int register_one_node(int nid)
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&node->access_list);
-	node_devices[nid] = node;
 
-	error = register_node(node_devices[nid], nid);
+	node->dev.id = nid;
+	node->dev.bus = &node_subsys;
+	node->dev.release = node_device_release;
+	node->dev.groups = node_dev_groups;
+
+	error = device_register(&node->dev);
 	if (error) {
-		node_devices[nid] = NULL;
+		put_device(&node->dev);
 		return error;
 	}
 
+	node_devices[nid] = node;
+	hugetlb_register_node(node);
+	compaction_register_node(node);
+	reclaim_register_node(node);
+
 	/* link cpu under this node */
 	for_each_present_cpu(cpu) {
 		if (cpu_to_node(cpu) == nid)
@@ -980,7 +968,7 @@ void __init node_dev_init(void)
 	 * to already created cpu devices.
 	 */
 	for_each_online_node(i) {
-		ret =  register_one_node(i);
+		ret =  register_node(i);
 		if (ret)
 			panic("%s() failed to add node: %d\n", __func__, ret);
 	}
diff --git a/include/linux/node.h b/include/linux/node.h
index 2c7529335b21..4dcf876cd0b4 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -168,7 +168,7 @@ static inline int hotplug_node_notifier(notifier_fn_t fn, int pri)
 #ifdef CONFIG_NUMA
 extern void node_dev_init(void);
 /* Core of the node registration - only memory hotplug should use this */
-extern int register_one_node(int nid);
+extern int register_node(int nid);
 extern void unregister_one_node(int nid);
 extern int register_cpu_under_node(unsigned int cpu, unsigned int nid);
 extern int unregister_cpu_under_node(unsigned int cpu, unsigned int nid);
@@ -181,7 +181,7 @@ extern int register_memory_node_under_compute_node(unsigned int mem_nid,
 static inline void node_dev_init(void)
 {
 }
-static inline int register_one_node(int nid)
+static inline int register_node(int nid)
 {
 	return 0;
 }
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 0be83039c3b5..6c050d867031 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1311,7 +1311,7 @@ static int __try_online_node(int nid, bool set_node_online)
 
 	if (set_node_online) {
 		node_set_online(nid);
-		ret = register_one_node(nid);
+		ret = register_node(nid);
 		BUG_ON(ret);
 	}
 out:
@@ -1542,7 +1542,7 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
 		goto error_memblock_remove;
 	if (ret) {
 		node_set_online(nid);
-		ret = register_one_node(nid);
+		ret = register_node(nid);
 		if (WARN_ON(ret)) {
 			node_set_offline(nid);
 			goto error_memblock_remove;
diff --git a/mm/mm_init.c b/mm/mm_init.c
index df614556741a..e1a19a3dadd7 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1909,7 +1909,7 @@ void __init free_area_init(unsigned long *max_zone_pfn)
 		free_area_init_node(nid);
 
 		/*
-		 * No sysfs hierarchy will be created via register_one_node()
+		 * No sysfs hierarchy will be created via register_node()
 		 *for memory-less node because here it's not marked as N_MEMORY
 		 *and won't be set online later. The benefit is userspace
 		 *program won't be confused by sysfs files/directories of
-- 
2.51.0

[PATCH 2/2] drivers/base/node: merge unregister_one_node() and unregister_node() to a single function.

[Donet Tom]

unregister_one_node() and unregister_node() are small functions.
This patch merges them into a single function named unregister_node()
to improve code readability.

No functional changes are introduced.

Signed-off-by: Donet Tom <donettom@linux.ibm.com>
---
 drivers/base/node.c  | 37 +++++++++++++++++--------------------
 include/linux/node.h |  6 ++----
 mm/memory_hotplug.c  |  4 ++--
 3 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index eab620e29c78..d460c1675c77 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -638,23 +638,6 @@ static void node_device_release(struct device *dev)
 	kfree(to_node(dev));
 }
 
-/**
- * unregister_node - unregister a node device
- * @node: node going away
- *
- * Unregisters a node device @node.  All the devices on the node must be
- * unregistered before calling this function.
- */
-void unregister_node(struct node *node)
-{
-	hugetlb_unregister_node(node);
-	compaction_unregister_node(node);
-	reclaim_unregister_node(node);
-	node_remove_accesses(node);
-	node_remove_caches(node);
-	device_unregister(&node->dev);
-}
-
 struct node *node_devices[MAX_NUMNODES];
 
 /*
@@ -887,12 +870,26 @@ int register_node(int nid)
 	return error;
 }
 
-void unregister_one_node(int nid)
+/**
+ * unregister_node - unregister a node device
+ * @nid: nid of the node going away
+ *
+ * Unregisters the node device at node id  @nid.  All the devices on the
+ * node must be unregistered before calling this function.
+ */
+void unregister_node(int nid)
 {
-	if (!node_devices[nid])
+	struct node *node = node_devices[nid];
+
+	if (!node)
 		return;
 
-	unregister_node(node_devices[nid]);
+	hugetlb_unregister_node(node);
+	compaction_unregister_node(node);
+	reclaim_unregister_node(node);
+	node_remove_accesses(node);
+	node_remove_caches(node);
+	device_unregister(&node->dev);
 	node_devices[nid] = NULL;
 }
 
diff --git a/include/linux/node.h b/include/linux/node.h
index 4dcf876cd0b4..d721127964b3 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -124,8 +124,6 @@ static inline void register_memory_blocks_under_nodes(void)
 }
 #endif
 
-extern void unregister_node(struct node *node);
-
 struct node_notify {
 	int nid;
 };
@@ -169,7 +167,7 @@ static inline int hotplug_node_notifier(notifier_fn_t fn, int pri)
 extern void node_dev_init(void);
 /* Core of the node registration - only memory hotplug should use this */
 extern int register_node(int nid);
-extern void unregister_one_node(int nid);
+extern void unregister_node(int nid);
 extern int register_cpu_under_node(unsigned int cpu, unsigned int nid);
 extern int unregister_cpu_under_node(unsigned int cpu, unsigned int nid);
 extern void unregister_memory_block_under_nodes(struct memory_block *mem_blk);
@@ -185,7 +183,7 @@ static inline int register_node(int nid)
 {
 	return 0;
 }
-static inline int unregister_one_node(int nid)
+static inline int unregister_node(int nid)
 {
 	return 0;
 }
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 6c050d867031..94a8f6e8811a 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1596,7 +1596,7 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
 error:
 	if (new_node) {
 		node_set_offline(nid);
-		unregister_one_node(nid);
+		unregister_node(nid);
 	}
 error_memblock_remove:
 	if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK))
@@ -2201,7 +2201,7 @@ void try_offline_node(int nid)
 	 * node now.
 	 */
 	node_set_offline(nid);
-	unregister_one_node(nid);
+	unregister_node(nid);
 }
 EXPORT_SYMBOL(try_offline_node);
 
-- 
2.51.0

Re: [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.

[Christophe Leroy]

Le 24/09/2025 à 20:40, Donet Tom a écrit :
> register_one_node() and register_node() are small functions.
> This patch merges them into a single function named register_node()
> to improve code readability.

This is unclear.

What it really does is it folds register_node() into its only caller 
which is register_one_node() and then it renames register_one_node() 
into register_node().

> 
> No functional changes are introduced.
> 
> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> ---
>   arch/powerpc/platforms/pseries/pci_dlpar.c |  2 +-
>   arch/x86/mm/numa.c                         |  4 +-
>   drivers/base/node.c                        | 52 +++++++++-------------
>   include/linux/node.h                       |  4 +-
>   mm/memory_hotplug.c                        |  4 +-
>   mm/mm_init.c                               |  2 +-
>   6 files changed, 28 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
> index aeb8633a3d00..8c77ec7980de 100644
> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
> @@ -29,7 +29,7 @@ struct pci_controller *init_phb_dynamic(struct device_node *dn)
>   	nid = of_node_to_nid(dn);
>   	if (likely((nid) >= 0)) {
>   		if (!node_online(nid)) {
> -			if (register_one_node(nid)) {
> +			if (register_node(nid)) {
>   				pr_err("PCI: Failed to register node %d\n", nid);
>   			} else {
>   				update_numa_distance(dn);
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index c24890c40138..7a97327140df 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -262,7 +262,7 @@ void __init init_gi_nodes(void)
>   	 * bringup_nonboot_cpus
>   	 *  cpu_up
>   	 *   __try_online_node
> -	 *    register_one_node
> +	 *    register_node
>   	 * because node_subsys is not initialized yet.
>   	 * TODO remove dependency on node_online
>   	 */
> @@ -303,7 +303,7 @@ void __init init_cpu_to_node(void)
>   		 * bringup_nonboot_cpus
>   		 *  cpu_up
>   		 *   __try_online_node
> -		 *    register_one_node
> +		 *    register_node
>   		 * because node_subsys is not initialized yet.
>   		 * TODO remove dependency on node_online
>   		 */
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 6b6e55a98b79..eab620e29c78 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -638,33 +638,6 @@ static void node_device_release(struct device *dev)
>   	kfree(to_node(dev));
>   }
>   
> -/*
> - * register_node - Setup a sysfs device for a node.
> - * @num - Node number to use when creating the device.
> - *
> - * Initialize and register the node device.
> - */
> -static int register_node(struct node *node, int num)
> -{
> -	int error;
> -
> -	node->dev.id = num;
> -	node->dev.bus = &node_subsys;
> -	node->dev.release = node_device_release;
> -	node->dev.groups = node_dev_groups;
> -	error = device_register(&node->dev);
> -
> -	if (error) {
> -		put_device(&node->dev);
> -	} else {
> -		hugetlb_register_node(node);
> -		compaction_register_node(node);
> -		reclaim_register_node(node);
> -	}
> -
> -	return error;
> -}
> -
>   /**
>    * unregister_node - unregister a node device
>    * @node: node going away
> @@ -869,7 +842,13 @@ void register_memory_blocks_under_node_hotplug(int nid, unsigned long start_pfn,
>   }
>   #endif /* CONFIG_MEMORY_HOTPLUG */
>   
> -int register_one_node(int nid)
> +/*
> + * register_node - Setup a sysfs device for a node.
> + * @nid - Node number to use when creating the device.
> + *
> + * Initialize and register the node device.
> + */
> +int register_node(int nid)
>   {
>   	int error;
>   	int cpu;
> @@ -880,14 +859,23 @@ int register_one_node(int nid)
>   		return -ENOMEM;
>   
>   	INIT_LIST_HEAD(&node->access_list);
> -	node_devices[nid] = node;
>   
> -	error = register_node(node_devices[nid], nid);
> +	node->dev.id = nid;
> +	node->dev.bus = &node_subsys;
> +	node->dev.release = node_device_release;
> +	node->dev.groups = node_dev_groups;
> +
> +	error = device_register(&node->dev);
>   	if (error) {
> -		node_devices[nid] = NULL;
> +		put_device(&node->dev);
>   		return error;
>   	}
>   
> +	node_devices[nid] = node;
> +	hugetlb_register_node(node);
> +	compaction_register_node(node);
> +	reclaim_register_node(node);
> +
>   	/* link cpu under this node */
>   	for_each_present_cpu(cpu) {
>   		if (cpu_to_node(cpu) == nid)
> @@ -980,7 +968,7 @@ void __init node_dev_init(void)
>   	 * to already created cpu devices.
>   	 */
>   	for_each_online_node(i) {
> -		ret =  register_one_node(i);
> +		ret =  register_node(i);
>   		if (ret)
>   			panic("%s() failed to add node: %d\n", __func__, ret);
>   	}
> diff --git a/include/linux/node.h b/include/linux/node.h
> index 2c7529335b21..4dcf876cd0b4 100644
> --- a/include/linux/node.h
> +++ b/include/linux/node.h
> @@ -168,7 +168,7 @@ static inline int hotplug_node_notifier(notifier_fn_t fn, int pri)
>   #ifdef CONFIG_NUMA
>   extern void node_dev_init(void);
>   /* Core of the node registration - only memory hotplug should use this */
> -extern int register_one_node(int nid);
> +extern int register_node(int nid);

extern keyword is pointless on functions prototypes.

checkpatch.pl usually complains about that.

I know previous prototype was extern and all surrounding also are, but 
it is not because mistakes were done in the past that you have to 
continue doing the same mistakes.

>   extern void unregister_one_node(int nid);
>   extern int register_cpu_under_node(unsigned int cpu, unsigned int nid);
>   extern int unregister_cpu_under_node(unsigned int cpu, unsigned int nid);
> @@ -181,7 +181,7 @@ extern int register_memory_node_under_compute_node(unsigned int mem_nid,
>   static inline void node_dev_init(void)
>   {
>   }
> -static inline int register_one_node(int nid)
> +static inline int register_node(int nid)
>   {
>   	return 0;
>   }
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 0be83039c3b5..6c050d867031 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1311,7 +1311,7 @@ static int __try_online_node(int nid, bool set_node_online)
>   
>   	if (set_node_online) {
>   		node_set_online(nid);
> -		ret = register_one_node(nid);
> +		ret = register_node(nid);
>   		BUG_ON(ret);
>   	}
>   out:
> @@ -1542,7 +1542,7 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
>   		goto error_memblock_remove;
>   	if (ret) {
>   		node_set_online(nid);
> -		ret = register_one_node(nid);
> +		ret = register_node(nid);
>   		if (WARN_ON(ret)) {
>   			node_set_offline(nid);
>   			goto error_memblock_remove;
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index df614556741a..e1a19a3dadd7 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -1909,7 +1909,7 @@ void __init free_area_init(unsigned long *max_zone_pfn)
>   		free_area_init_node(nid);
>   
>   		/*
> -		 * No sysfs hierarchy will be created via register_one_node()
> +		 * No sysfs hierarchy will be created via register_node()
>   		 *for memory-less node because here it's not marked as N_MEMORY
>   		 *and won't be set online later. The benefit is userspace
>   		 *program won't be confused by sysfs files/directories of

Re: [PATCH 2/2] drivers/base/node: merge unregister_one_node() and unregister_node() to a single function.

[Christophe Leroy]

Le 24/09/2025 à 20:40, Donet Tom a écrit :
> unregister_one_node() and unregister_node() are small functions.
> This patch merges them into a single function named unregister_node()
> to improve code readability.

Same comment than patch 1. It is not only because they are small that 
you merge them, it is primarily because unregister_node() only has one 
caller.

> 
> No functional changes are introduced.
> 
> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> ---
>   drivers/base/node.c  | 37 +++++++++++++++++--------------------
>   include/linux/node.h |  6 ++----
>   mm/memory_hotplug.c  |  4 ++--
>   3 files changed, 21 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index eab620e29c78..d460c1675c77 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -638,23 +638,6 @@ static void node_device_release(struct device *dev)
>   	kfree(to_node(dev));
>   }
>   
> -/**
> - * unregister_node - unregister a node device
> - * @node: node going away
> - *
> - * Unregisters a node device @node.  All the devices on the node must be
> - * unregistered before calling this function.
> - */
> -void unregister_node(struct node *node)
> -{
> -	hugetlb_unregister_node(node);
> -	compaction_unregister_node(node);
> -	reclaim_unregister_node(node);
> -	node_remove_accesses(node);
> -	node_remove_caches(node);
> -	device_unregister(&node->dev);
> -}
> -
>   struct node *node_devices[MAX_NUMNODES];
>   
>   /*
> @@ -887,12 +870,26 @@ int register_node(int nid)
>   	return error;
>   }
>   
> -void unregister_one_node(int nid)
> +/**
> + * unregister_node - unregister a node device
> + * @nid: nid of the node going away
> + *
> + * Unregisters the node device at node id  @nid.  All the devices on the
> + * node must be unregistered before calling this function.
> + */
> +void unregister_node(int nid)
>   {
> -	if (!node_devices[nid])
> +	struct node *node = node_devices[nid];
> +
> +	if (!node)
>   		return;
>   
> -	unregister_node(node_devices[nid]);
> +	hugetlb_unregister_node(node);
> +	compaction_unregister_node(node);
> +	reclaim_unregister_node(node);
> +	node_remove_accesses(node);
> +	node_remove_caches(node);
> +	device_unregister(&node->dev);
>   	node_devices[nid] = NULL;
>   }
>   
> diff --git a/include/linux/node.h b/include/linux/node.h
> index 4dcf876cd0b4..d721127964b3 100644
> --- a/include/linux/node.h
> +++ b/include/linux/node.h
> @@ -124,8 +124,6 @@ static inline void register_memory_blocks_under_nodes(void)
>   }
>   #endif
>   
> -extern void unregister_node(struct node *node);
> -
>   struct node_notify {
>   	int nid;
>   };
> @@ -169,7 +167,7 @@ static inline int hotplug_node_notifier(notifier_fn_t fn, int pri)
>   extern void node_dev_init(void);
>   /* Core of the node registration - only memory hotplug should use this */
>   extern int register_node(int nid);
> -extern void unregister_one_node(int nid);
> +extern void unregister_node(int nid);

No 'extern' on function prototypes.

>   extern int register_cpu_under_node(unsigned int cpu, unsigned int nid);
>   extern int unregister_cpu_under_node(unsigned int cpu, unsigned int nid);
>   extern void unregister_memory_block_under_nodes(struct memory_block *mem_blk);
> @@ -185,7 +183,7 @@ static inline int register_node(int nid)
>   {
>   	return 0;
>   }
> -static inline int unregister_one_node(int nid)
> +static inline int unregister_node(int nid)
>   {
>   	return 0;
>   }
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 6c050d867031..94a8f6e8811a 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1596,7 +1596,7 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
>   error:
>   	if (new_node) {
>   		node_set_offline(nid);
> -		unregister_one_node(nid);
> +		unregister_node(nid);
>   	}
>   error_memblock_remove:
>   	if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK))
> @@ -2201,7 +2201,7 @@ void try_offline_node(int nid)
>   	 * node now.
>   	 */
>   	node_set_offline(nid);
> -	unregister_one_node(nid);
> +	unregister_node(nid);
>   }
>   EXPORT_SYMBOL(try_offline_node);
>   

Re: [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.

[Donet Tom]

On 9/25/25 12:45 AM, Christophe Leroy wrote:
>
>
> Le 24/09/2025 à 20:40, Donet Tom a écrit :
>> register_one_node() and register_node() are small functions.
>> This patch merges them into a single function named register_node()
>> to improve code readability.
>
> This is unclear.
>
> What it really does is it folds register_node() into its only caller 
> which is register_one_node() and then it renames register_one_node() 
> into register_node().

Yes, you are right. I will add proper explanations in the next version.

>
>>
>> No functional changes are introduced.
>>
>> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
>> ---
>>   arch/powerpc/platforms/pseries/pci_dlpar.c |  2 +-
>>   arch/x86/mm/numa.c                         |  4 +-
>>   drivers/base/node.c                        | 52 +++++++++-------------
>>   include/linux/node.h                       |  4 +-
>>   mm/memory_hotplug.c                        |  4 +-
>>   mm/mm_init.c                               |  2 +-
>>   6 files changed, 28 insertions(+), 40 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c 
>> b/arch/powerpc/platforms/pseries/pci_dlpar.c
>> index aeb8633a3d00..8c77ec7980de 100644
>> --- a/arch/powerpc/platforms/pseries/pci_dlpar.c
>> +++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
>> @@ -29,7 +29,7 @@ struct pci_controller *init_phb_dynamic(struct 
>> device_node *dn)
>>       nid = of_node_to_nid(dn);
>>       if (likely((nid) >= 0)) {
>>           if (!node_online(nid)) {
>> -            if (register_one_node(nid)) {
>> +            if (register_node(nid)) {
>>                   pr_err("PCI: Failed to register node %d\n", nid);
>>               } else {
>>                   update_numa_distance(dn);
>> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
>> index c24890c40138..7a97327140df 100644
>> --- a/arch/x86/mm/numa.c
>> +++ b/arch/x86/mm/numa.c
>> @@ -262,7 +262,7 @@ void __init init_gi_nodes(void)
>>        * bringup_nonboot_cpus
>>        *  cpu_up
>>        *   __try_online_node
>> -     *    register_one_node
>> +     *    register_node
>>        * because node_subsys is not initialized yet.
>>        * TODO remove dependency on node_online
>>        */
>> @@ -303,7 +303,7 @@ void __init init_cpu_to_node(void)
>>            * bringup_nonboot_cpus
>>            *  cpu_up
>>            *   __try_online_node
>> -         *    register_one_node
>> +         *    register_node
>>            * because node_subsys is not initialized yet.
>>            * TODO remove dependency on node_online
>>            */
>> diff --git a/drivers/base/node.c b/drivers/base/node.c
>> index 6b6e55a98b79..eab620e29c78 100644
>> --- a/drivers/base/node.c
>> +++ b/drivers/base/node.c
>> @@ -638,33 +638,6 @@ static void node_device_release(struct device *dev)
>>       kfree(to_node(dev));
>>   }
>>   -/*
>> - * register_node - Setup a sysfs device for a node.
>> - * @num - Node number to use when creating the device.
>> - *
>> - * Initialize and register the node device.
>> - */
>> -static int register_node(struct node *node, int num)
>> -{
>> -    int error;
>> -
>> -    node->dev.id = num;
>> -    node->dev.bus = &node_subsys;
>> -    node->dev.release = node_device_release;
>> -    node->dev.groups = node_dev_groups;
>> -    error = device_register(&node->dev);
>> -
>> -    if (error) {
>> -        put_device(&node->dev);
>> -    } else {
>> -        hugetlb_register_node(node);
>> -        compaction_register_node(node);
>> -        reclaim_register_node(node);
>> -    }
>> -
>> -    return error;
>> -}
>> -
>>   /**
>>    * unregister_node - unregister a node device
>>    * @node: node going away
>> @@ -869,7 +842,13 @@ void 
>> register_memory_blocks_under_node_hotplug(int nid, unsigned long 
>> start_pfn,
>>   }
>>   #endif /* CONFIG_MEMORY_HOTPLUG */
>>   -int register_one_node(int nid)
>> +/*
>> + * register_node - Setup a sysfs device for a node.
>> + * @nid - Node number to use when creating the device.
>> + *
>> + * Initialize and register the node device.
>> + */
>> +int register_node(int nid)
>>   {
>>       int error;
>>       int cpu;
>> @@ -880,14 +859,23 @@ int register_one_node(int nid)
>>           return -ENOMEM;
>>         INIT_LIST_HEAD(&node->access_list);
>> -    node_devices[nid] = node;
>>   -    error = register_node(node_devices[nid], nid);
>> +    node->dev.id = nid;
>> +    node->dev.bus = &node_subsys;
>> +    node->dev.release = node_device_release;
>> +    node->dev.groups = node_dev_groups;
>> +
>> +    error = device_register(&node->dev);
>>       if (error) {
>> -        node_devices[nid] = NULL;
>> +        put_device(&node->dev);
>>           return error;
>>       }
>>   +    node_devices[nid] = node;
>> +    hugetlb_register_node(node);
>> +    compaction_register_node(node);
>> +    reclaim_register_node(node);
>> +
>>       /* link cpu under this node */
>>       for_each_present_cpu(cpu) {
>>           if (cpu_to_node(cpu) == nid)
>> @@ -980,7 +968,7 @@ void __init node_dev_init(void)
>>        * to already created cpu devices.
>>        */
>>       for_each_online_node(i) {
>> -        ret =  register_one_node(i);
>> +        ret =  register_node(i);
>>           if (ret)
>>               panic("%s() failed to add node: %d\n", __func__, ret);
>>       }
>> diff --git a/include/linux/node.h b/include/linux/node.h
>> index 2c7529335b21..4dcf876cd0b4 100644
>> --- a/include/linux/node.h
>> +++ b/include/linux/node.h
>> @@ -168,7 +168,7 @@ static inline int 
>> hotplug_node_notifier(notifier_fn_t fn, int pri)
>>   #ifdef CONFIG_NUMA
>>   extern void node_dev_init(void);
>>   /* Core of the node registration - only memory hotplug should use 
>> this */
>> -extern int register_one_node(int nid);
>> +extern int register_node(int nid);
>
> extern keyword is pointless on functions prototypes.
>
> checkpatch.pl usually complains about that.
>
> I know previous prototype was extern and all surrounding also are, but 
> it is not because mistakes were done in the past that you have to 
> continue doing the same mistakes.
>

Thank you for the comment. I will address it in the next version.


>
>
>>   extern void unregister_one_node(int nid);
>>   extern int register_cpu_under_node(unsigned int cpu, unsigned int 
>> nid);
>>   extern int unregister_cpu_under_node(unsigned int cpu, unsigned int 
>> nid);
>> @@ -181,7 +181,7 @@ extern int 
>> register_memory_node_under_compute_node(unsigned int mem_nid,
>>   static inline void node_dev_init(void)
>>   {
>>   }
>> -static inline int register_one_node(int nid)
>> +static inline int register_node(int nid)
>>   {
>>       return 0;
>>   }
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 0be83039c3b5..6c050d867031 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1311,7 +1311,7 @@ static int __try_online_node(int nid, bool 
>> set_node_online)
>>         if (set_node_online) {
>>           node_set_online(nid);
>> -        ret = register_one_node(nid);
>> +        ret = register_node(nid);
>>           BUG_ON(ret);
>>       }
>>   out:
>> @@ -1542,7 +1542,7 @@ int add_memory_resource(int nid, struct 
>> resource *res, mhp_t mhp_flags)
>>           goto error_memblock_remove;
>>       if (ret) {
>>           node_set_online(nid);
>> -        ret = register_one_node(nid);
>> +        ret = register_node(nid);
>>           if (WARN_ON(ret)) {
>>               node_set_offline(nid);
>>               goto error_memblock_remove;
>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>> index df614556741a..e1a19a3dadd7 100644
>> --- a/mm/mm_init.c
>> +++ b/mm/mm_init.c
>> @@ -1909,7 +1909,7 @@ void __init free_area_init(unsigned long 
>> *max_zone_pfn)
>>           free_area_init_node(nid);
>>             /*
>> -         * No sysfs hierarchy will be created via register_one_node()
>> +         * No sysfs hierarchy will be created via register_node()
>>            *for memory-less node because here it's not marked as 
>> N_MEMORY
>>            *and won't be set online later. The benefit is userspace
>>            *program won't be confused by sysfs files/directories of
>
>

Re: [PATCH 2/2] drivers/base/node: merge unregister_one_node() and unregister_node() to a single function.

[Donet Tom]

On 9/25/25 12:49 AM, Christophe Leroy wrote:
>
>
> Le 24/09/2025 à 20:40, Donet Tom a écrit :
>> unregister_one_node() and unregister_node() are small functions.
>> This patch merges them into a single function named unregister_node()
>> to improve code readability.
>
> Same comment than patch 1. It is not only because they are small that 
> you merge them, it is primarily because unregister_node() only has one 
> caller.


Will update it in the next version.


>
>>
>> No functional changes are introduced.
>>
>> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
>> ---
>>   drivers/base/node.c  | 37 +++++++++++++++++--------------------
>>   include/linux/node.h |  6 ++----
>>   mm/memory_hotplug.c  |  4 ++--
>>   3 files changed, 21 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/base/node.c b/drivers/base/node.c
>> index eab620e29c78..d460c1675c77 100644
>> --- a/drivers/base/node.c
>> +++ b/drivers/base/node.c
>> @@ -638,23 +638,6 @@ static void node_device_release(struct device *dev)
>>       kfree(to_node(dev));
>>   }
>>   -/**
>> - * unregister_node - unregister a node device
>> - * @node: node going away
>> - *
>> - * Unregisters a node device @node.  All the devices on the node 
>> must be
>> - * unregistered before calling this function.
>> - */
>> -void unregister_node(struct node *node)
>> -{
>> -    hugetlb_unregister_node(node);
>> -    compaction_unregister_node(node);
>> -    reclaim_unregister_node(node);
>> -    node_remove_accesses(node);
>> -    node_remove_caches(node);
>> -    device_unregister(&node->dev);
>> -}
>> -
>>   struct node *node_devices[MAX_NUMNODES];
>>     /*
>> @@ -887,12 +870,26 @@ int register_node(int nid)
>>       return error;
>>   }
>>   -void unregister_one_node(int nid)
>> +/**
>> + * unregister_node - unregister a node device
>> + * @nid: nid of the node going away
>> + *
>> + * Unregisters the node device at node id  @nid.  All the devices on 
>> the
>> + * node must be unregistered before calling this function.
>> + */
>> +void unregister_node(int nid)
>>   {
>> -    if (!node_devices[nid])
>> +    struct node *node = node_devices[nid];
>> +
>> +    if (!node)
>>           return;
>>   -    unregister_node(node_devices[nid]);
>> +    hugetlb_unregister_node(node);
>> +    compaction_unregister_node(node);
>> +    reclaim_unregister_node(node);
>> +    node_remove_accesses(node);
>> +    node_remove_caches(node);
>> +    device_unregister(&node->dev);
>>       node_devices[nid] = NULL;
>>   }
>>   diff --git a/include/linux/node.h b/include/linux/node.h
>> index 4dcf876cd0b4..d721127964b3 100644
>> --- a/include/linux/node.h
>> +++ b/include/linux/node.h
>> @@ -124,8 +124,6 @@ static inline void 
>> register_memory_blocks_under_nodes(void)
>>   }
>>   #endif
>>   -extern void unregister_node(struct node *node);
>> -
>>   struct node_notify {
>>       int nid;
>>   };
>> @@ -169,7 +167,7 @@ static inline int 
>> hotplug_node_notifier(notifier_fn_t fn, int pri)
>>   extern void node_dev_init(void);
>>   /* Core of the node registration - only memory hotplug should use 
>> this */
>>   extern int register_node(int nid);
>> -extern void unregister_one_node(int nid);
>> +extern void unregister_node(int nid);
>
> No 'extern' on function prototypes.


Sure.

Thank you very much


>
>>   extern int register_cpu_under_node(unsigned int cpu, unsigned int 
>> nid);
>>   extern int unregister_cpu_under_node(unsigned int cpu, unsigned int 
>> nid);
>>   extern void unregister_memory_block_under_nodes(struct memory_block 
>> *mem_blk);
>> @@ -185,7 +183,7 @@ static inline int register_node(int nid)
>>   {
>>       return 0;
>>   }
>> -static inline int unregister_one_node(int nid)
>> +static inline int unregister_node(int nid)
>>   {
>>       return 0;
>>   }
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index 6c050d867031..94a8f6e8811a 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1596,7 +1596,7 @@ int add_memory_resource(int nid, struct 
>> resource *res, mhp_t mhp_flags)
>>   error:
>>       if (new_node) {
>>           node_set_offline(nid);
>> -        unregister_one_node(nid);
>> +        unregister_node(nid);
>>       }
>>   error_memblock_remove:
>>       if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK))
>> @@ -2201,7 +2201,7 @@ void try_offline_node(int nid)
>>        * node now.
>>        */
>>       node_set_offline(nid);
>> -    unregister_one_node(nid);
>> +    unregister_node(nid);
>>   }
>>   EXPORT_SYMBOL(try_offline_node);
>
>

Re: [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.

[David Hildenbrand]

On 24.09.25 20:40, Donet Tom wrote:
> register_one_node() and register_node() are small functions.
> This patch merges them into a single function named register_node()
> to improve code readability.
> 
> No functional changes are introduced.
> 
> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> ---

[...]

>   /**
>    * unregister_node - unregister a node device
>    * @node: node going away
> @@ -869,7 +842,13 @@ void register_memory_blocks_under_node_hotplug(int nid, unsigned long start_pfn,
>   }
>   #endif /* CONFIG_MEMORY_HOTPLUG */
>   
> -int register_one_node(int nid)
> +/*

We can directly convert this to proper kernel doc by using /**

> + * register_node - Setup a sysfs device for a node.
> + * @nid - Node number to use when creating the device.
> + *
> + * Initialize and register the node device.

and briefly describing what the return value means

"Returns 0 on success, ..."

> + */
> +int register_node(int nid)
>   {
>   	int error;
>   	int cpu;
> @@ -880,14 +859,23 @@ int register_one_node(int nid)
>   		return -ENOMEM;
>   
>   	INIT_LIST_HEAD(&node->access_list);
> -	node_devices[nid] = node;
>   
> -	error = register_node(node_devices[nid], nid);
> +	node->dev.id = nid;
> +	node->dev.bus = &node_subsys;
> +	node->dev.release = node_device_release;
> +	node->dev.groups = node_dev_groups;
> +
> +	error = device_register(&node->dev);
>   	if (error) {
> -		node_devices[nid] = NULL;

Wondering why we did have this temporary setting of the node_devices[] 
in there. But I cannot immediately spot why it was required.

-- 
Cheers

David / dhildenb

Re: [PATCH 2/2] drivers/base/node: merge unregister_one_node() and unregister_node() to a single function.

[David Hildenbrand]

On 24.09.25 20:40, Donet Tom wrote:
> unregister_one_node() and unregister_node() are small functions.
> This patch merges them into a single function named unregister_node()
> to improve code readability.
> 
> No functional changes are introduced.
> 
> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> ---

LGTM with the description adjusted and "extern" dropped

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.

[Mike Rapoport]

On Thu, Sep 25, 2025 at 10:54:07AM +0200, David Hildenbrand wrote:
> On 24.09.25 20:40, Donet Tom wrote:
> > register_one_node() and register_node() are small functions.
> > This patch merges them into a single function named register_node()
> > to improve code readability.
> > 
> > No functional changes are introduced.
> > 
> > Signed-off-by: Donet Tom <donettom@linux.ibm.com>
> > ---
> 
> [...]
> 
> >   /**
> >    * unregister_node - unregister a node device
> >    * @node: node going away
> > @@ -869,7 +842,13 @@ void register_memory_blocks_under_node_hotplug(int nid, unsigned long start_pfn,
> >   }
> >   #endif /* CONFIG_MEMORY_HOTPLUG */
> > -int register_one_node(int nid)
> > +/*
> 
> We can directly convert this to proper kernel doc by using /**
> 
> > + * register_node - Setup a sysfs device for a node.
> > + * @nid - Node number to use when creating the device.
> > + *
> > + * Initialize and register the node device.
> 
> and briefly describing what the return value means
> 
> "Returns 0 on success, ..."

For kernel-doc it should be

Return: 0 on success, ...

> 
> > + */
> > +int register_node(int nid)
> >   {
> >   	int error;
> >   	int cpu;
> > @@ -880,14 +859,23 @@ int register_one_node(int nid)
> >   		return -ENOMEM;
> >   	INIT_LIST_HEAD(&node->access_list);
> > -	node_devices[nid] = node;
> > -	error = register_node(node_devices[nid], nid);
> > +	node->dev.id = nid;
> > +	node->dev.bus = &node_subsys;
> > +	node->dev.release = node_device_release;
> > +	node->dev.groups = node_dev_groups;
> > +
> > +	error = device_register(&node->dev);
> >   	if (error) {
> > -		node_devices[nid] = NULL;
> 
> Wondering why we did have this temporary setting of the node_devices[] in
> there. But I cannot immediately spot why it was required.

register_cpu_under_node() references node_devices, so that assignment can
be moved just before the loop that adds CPUs to node.
 
> -- 
> Cheers
> 
> David / dhildenb
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 0/2] drivers/base/node: fold node register and unregister functions

[Mike Rapoport]

On Thu, Sep 25, 2025 at 12:10:49AM +0530, Donet Tom wrote:
> This change came from the discussion in [1]. It consists of two patches:
> 
> The first patch merges register_one_node() and register_node(), leaving a
> single register_node() function.
> 
> The second patch merges unregister_one_node() and unregister_node(), leaving
> a single unregister_node() function.
> 
> There is no functional change in these patches.
> 
> [1] https://lore.kernel.org/all/5512b1b6-31f8-4322-8a5f-add8d1e9b22f@redhat.com/
> 
> Donet Tom (2):
>   drivers/base/node: merge register_one_node() and register_node() to a
>     single function.
>   drivers/base/node: merge unregister_one_node() and unregister_node()
>     to a single function.

Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

>  arch/powerpc/platforms/pseries/pci_dlpar.c |  2 +-
>  arch/x86/mm/numa.c                         |  4 +-
>  drivers/base/node.c                        | 89 +++++++++-------------
>  include/linux/node.h                       | 10 +--
>  mm/memory_hotplug.c                        |  8 +-
>  mm/mm_init.c                               |  2 +-
>  6 files changed, 49 insertions(+), 66 deletions(-)
> 
> -- 
> 2.51.0
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.

[David Hildenbrand]

On 25.09.25 11:34, Mike Rapoport wrote:
> On Thu, Sep 25, 2025 at 10:54:07AM +0200, David Hildenbrand wrote:
>> On 24.09.25 20:40, Donet Tom wrote:
>>> register_one_node() and register_node() are small functions.
>>> This patch merges them into a single function named register_node()
>>> to improve code readability.
>>>
>>> No functional changes are introduced.
>>>
>>> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
>>> ---
>>
>> [...]
>>
>>>    /**
>>>     * unregister_node - unregister a node device
>>>     * @node: node going away
>>> @@ -869,7 +842,13 @@ void register_memory_blocks_under_node_hotplug(int nid, unsigned long start_pfn,
>>>    }
>>>    #endif /* CONFIG_MEMORY_HOTPLUG */
>>> -int register_one_node(int nid)
>>> +/*
>>
>> We can directly convert this to proper kernel doc by using /**
>>
>>> + * register_node - Setup a sysfs device for a node.
>>> + * @nid - Node number to use when creating the device.
>>> + *
>>> + * Initialize and register the node device.
>>
>> and briefly describing what the return value means
>>
>> "Returns 0 on success, ..."
> 
> For kernel-doc it should be
> 
> Return: 0 on success, ...

Yeah; I recall that kerneldoc does not complain when using "Returns 
...", but probably it will not be indicated accordingly.

-- 
Cheers

David / dhildenb

Re: [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.

[Donet Tom]

On 9/25/25 2:24 PM, David Hildenbrand wrote:
> On 24.09.25 20:40, Donet Tom wrote:
>> register_one_node() and register_node() are small functions.
>> This patch merges them into a single function named register_node()
>> to improve code readability.
>>
>> No functional changes are introduced.
>>
>> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
>> ---
>
> [...]
>
>>   /**
>>    * unregister_node - unregister a node device
>>    * @node: node going away
>> @@ -869,7 +842,13 @@ void 
>> register_memory_blocks_under_node_hotplug(int nid, unsigned long 
>> start_pfn,
>>   }
>>   #endif /* CONFIG_MEMORY_HOTPLUG */
>>   -int register_one_node(int nid)
>> +/*
>
> We can directly convert this to proper kernel doc by using /**
>
Sure I will add it.


>
>> + * register_node - Setup a sysfs device for a node.
>> + * @nid - Node number to use when creating the device.
>> + *
>> + * Initialize and register the node device.
>
> and briefly describing what the return value means
>
> "Returns 0 on success, ..."
>
Sure

>> + */
>> +int register_node(int nid)
>>   {
>>       int error;
>>       int cpu;
>> @@ -880,14 +859,23 @@ int register_one_node(int nid)
>>           return -ENOMEM;
>>         INIT_LIST_HEAD(&node->access_list);
>> -    node_devices[nid] = node;
>>   -    error = register_node(node_devices[nid], nid);
>> +    node->dev.id = nid;
>> +    node->dev.bus = &node_subsys;
>> +    node->dev.release = node_device_release;
>> +    node->dev.groups = node_dev_groups;
>> +
>> +    error = device_register(&node->dev);
>>       if (error) {
>> -        node_devices[nid] = NULL;
>
> Wondering why we did have this temporary setting of the node_devices[] 
> in there. But I cannot immediately spot why it was required.


node_devices[] is used in many places to access node data.

In the previous code, immediately after allocating the node

structure, it was stored in node_devices[]. On error paths, we

were clearing the node structure entry from node_devices[].


With the new code, the node structure is now stored in node_devices[]

only after the device has been registered, and just before calling

register_cpu_under_node(), since node_devices[] is accessed inside

that function.


It is also used outside of node.c, in hugetlb_register_all_nodes() .

Do you think we could use a different mechanism instead of this?

Re: [PATCH 1/2] drivers/base/node: merge register_one_node() and register_node() to a single function.

[Donet Tom]

On 9/25/25 3:04 PM, Mike Rapoport wrote:
> On Thu, Sep 25, 2025 at 10:54:07AM +0200, David Hildenbrand wrote:
>> On 24.09.25 20:40, Donet Tom wrote:
>>> register_one_node() and register_node() are small functions.
>>> This patch merges them into a single function named register_node()
>>> to improve code readability.
>>>
>>> No functional changes are introduced.
>>>
>>> Signed-off-by: Donet Tom <donettom@linux.ibm.com>
>>> ---
>> [...]
>>
>>>    /**
>>>     * unregister_node - unregister a node device
>>>     * @node: node going away
>>> @@ -869,7 +842,13 @@ void register_memory_blocks_under_node_hotplug(int nid, unsigned long start_pfn,
>>>    }
>>>    #endif /* CONFIG_MEMORY_HOTPLUG */
>>> -int register_one_node(int nid)
>>> +/*
>> We can directly convert this to proper kernel doc by using /**
>>
>>> + * register_node - Setup a sysfs device for a node.
>>> + * @nid - Node number to use when creating the device.
>>> + *
>>> + * Initialize and register the node device.
>> and briefly describing what the return value means
>>
>> "Returns 0 on success, ..."
> For kernel-doc it should be
>
> Return: 0 on success, ...
>

Sure I will change it.


>
>>> + */
>>> +int register_node(int nid)
>>>    {
>>>    	int error;
>>>    	int cpu;
>>> @@ -880,14 +859,23 @@ int register_one_node(int nid)
>>>    		return -ENOMEM;
>>>    	INIT_LIST_HEAD(&node->access_list);
>>> -	node_devices[nid] = node;
>>> -	error = register_node(node_devices[nid], nid);
>>> +	node->dev.id = nid;
>>> +	node->dev.bus = &node_subsys;
>>> +	node->dev.release = node_device_release;
>>> +	node->dev.groups = node_dev_groups;
>>> +
>>> +	error = device_register(&node->dev);
>>>    	if (error) {
>>> -		node_devices[nid] = NULL;
>> Wondering why we did have this temporary setting of the node_devices[] in
>> there. But I cannot immediately spot why it was required.
> register_cpu_under_node() references node_devices, so that assignment can
> be moved just before the loop that adds CPUs to node.


Sure.

Thank you


>   
>> -- 
>> Cheers
>>
>> David / dhildenb
>>