Re: [PATCH 3/3] drivers/base: move memory_block_add_nid() into the caller

[Donet Tom <donettom@linux.ibm.com>]

On 7/4/25 12:04 PM, Hannes Reinecke wrote:
> Now the node id only needs to be set for early memory, so move
> memory_block_add_nid() into the caller and rename it into
> memory_block_add_nid_early(). This allows us to further simplify
> the code by dropping the 'context' argument to
> do_register_memory_block_under_node().
>
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> Acked-by: David Hildenbrand <david@redhat.com>
> ---
>   drivers/base/memory.c  | 36 ++++++++++++++++++------------------
>   drivers/base/node.c    | 10 ++++------
>   include/linux/memory.h |  3 +--
>   3 files changed, 23 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 4ce93fb54525..5fb275d7ed7d 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -782,21 +782,22 @@ static struct zone *early_node_zone_for_memory_block(struct memory_block *mem,
>   
>   #ifdef CONFIG_NUMA
>   /**
> - * memory_block_add_nid() - Indicate that system RAM falling into this memory
> - *			    block device (partially) belongs to the given node.
> + * memory_block_add_nid_early() - Indicate that early system RAM falling into
> + *				  this memory block device (partially) belongs
> + *				  to the given node.
>    * @mem: The memory block device.
>    * @nid: The node id.
> - * @context: The memory initialization context.
>    *
> - * Indicate that system RAM falling into this memory block (partially) belongs
> - * to the given node. If the context indicates ("early") that we are adding the
> - * node during node device subsystem initialization, this will also properly
> - * set/adjust mem->zone based on the zone ranges of the given node.
> + * Indicate that early system RAM falling into this memory block (partially)
> + * belongs to the given node. This will also properly set/adjust mem->zone based
> + * on the zone ranges of the given node.
> + *
> + * Memory hotplug handles this on memory block creation, where we can only have
> + * a single nid span a memory block.
>    */
> -void memory_block_add_nid(struct memory_block *mem, int nid,
> -			  enum meminit_context context)
> +void memory_block_add_nid_early(struct memory_block *mem, int nid)
>   {
> -	if (context == MEMINIT_EARLY && mem->nid != nid) {
> +	if (mem->nid != nid) {
>   		/*
>   		 * For early memory we have to determine the zone when setting
>   		 * the node id and handle multiple nodes spanning a single
> @@ -810,15 +811,14 @@ void memory_block_add_nid(struct memory_block *mem, int nid,
>   			mem->zone = early_node_zone_for_memory_block(mem, nid);
>   		else
>   			mem->zone = NULL;
> +		/*
> +		 * If this memory block spans multiple nodes, we only indicate
> +		 * the last processed node. If we span multiple nodes (not applicable
> +		 * to hotplugged memory), zone == NULL will prohibit memory offlining
> +		 * and consequently unplug.
> +		 */
> +		mem->nid = nid;
>   	}
> -
> -	/*
> -	 * If this memory block spans multiple nodes, we only indicate
> -	 * the last processed node. If we span multiple nodes (not applicable
> -	 * to hotplugged memory), zone == NULL will prohibit memory offlining
> -	 * and consequently unplug.
> -	 */
> -	mem->nid = nid;
>   }
>   #endif
>   
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index c19094481630..8e3225434ae7 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -766,13 +766,10 @@ static int __ref get_nid_for_pfn(unsigned long pfn)
>   }
>   
>   static void do_register_memory_block_under_node(int nid,
> -						struct memory_block *mem_blk,
> -						enum meminit_context context)
> +						struct memory_block *mem_blk)
>   {
>   	int ret;
>   
> -	memory_block_add_nid(mem_blk, nid, context);
> -
>   	ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
>   				       &mem_blk->dev.kobj,
>   				       kobject_name(&mem_blk->dev.kobj));
> @@ -824,7 +821,8 @@ static int register_mem_block_under_node_early(struct memory_block *mem_blk,
>   		if (page_nid != nid)
>   			continue;
>   
> -		do_register_memory_block_under_node(nid, mem_blk, MEMINIT_EARLY);
> +		memory_block_add_nid_early(mem_blk, nid);
> +		do_register_memory_block_under_node(nid, mem_blk);
>   		return 0;
>   	}
>   	/* mem section does not span the specified node */
> @@ -840,7 +838,7 @@ static int register_mem_block_under_node_hotplug(struct memory_block *mem_blk,
>   {
>   	int nid = *(int *)arg;
>   
> -	do_register_memory_block_under_node(nid, mem_blk, MEMINIT_HOTPLUG);
> +	do_register_memory_block_under_node(nid, mem_blk);
>   	return 0;
>   }
>   
> diff --git a/include/linux/memory.h b/include/linux/memory.h
> index d7c3a4856031..63f076f2d303 100644
> --- a/include/linux/memory.h
> +++ b/include/linux/memory.h
> @@ -186,8 +186,7 @@ int walk_dynamic_memory_groups(int nid, walk_memory_groups_func_t func,
>   })
>   
>   #ifdef CONFIG_NUMA
> -void memory_block_add_nid(struct memory_block *mem, int nid,
> -			  enum meminit_context context);
> +void memory_block_add_nid_early(struct memory_block *mem, int nid);
>   #endif /* CONFIG_NUMA */
>   int memory_block_advise_max_size(unsigned long size);
>   unsigned long memory_block_advised_max_size(void);



This looks good to me.

Reviewed-by: Donet Tom <donettom@linux.ibm.com>