Encapsulate the mhp_init_memmap_on_memory() and online_pages() into
online_memory_block_pages(). Thus we can further optimize the
set_zone_contiguous() to check the whole memory block range, instead
of check the zone contiguous in separate range.

Correspondingly, encapsulate the mhp_deinit_memmap_on_memory() and
offline_pages() into offline_memory_block_pages().

Tested-by: Yuan Liu <yuan1.liu@intel.com>
Reviewed-by: Yuan Liu <yuan1.liu@intel.com>
Signed-off-by: Tianyou Li <tianyou.li@intel.com>
---
 drivers/base/memory.c          | 53 ++++++---------------------
 include/linux/memory_hotplug.h | 18 +++++-----
 mm/memory_hotplug.c            | 65 +++++++++++++++++++++++++++++++---
 3 files changed, 80 insertions(+), 56 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 751f248ca4a8..ea4d6fbf34fd 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -246,31 +246,12 @@ static int memory_block_online(struct memory_block *mem)
 		nr_vmemmap_pages = mem->altmap->free;
 
 	mem_hotplug_begin();
-	if (nr_vmemmap_pages) {
-		ret = mhp_init_memmap_on_memory(start_pfn, nr_vmemmap_pages, zone);
-		if (ret)
-			goto out;
-	}
-
-	ret = online_pages(start_pfn + nr_vmemmap_pages,
-			   nr_pages - nr_vmemmap_pages, zone, mem->group);
-	if (ret) {
-		if (nr_vmemmap_pages)
-			mhp_deinit_memmap_on_memory(start_pfn, nr_vmemmap_pages);
-		goto out;
-	}
-
-	/*
-	 * Account once onlining succeeded. If the zone was unpopulated, it is
-	 * now already properly populated.
-	 */
-	if (nr_vmemmap_pages)
-		adjust_present_page_count(pfn_to_page(start_pfn), mem->group,
-					  nr_vmemmap_pages);
-
-	mem->zone = zone;
-out:
+	ret = online_memory_block_pages(start_pfn, nr_pages, nr_vmemmap_pages,
+					zone, mem->group);
+	if (!ret)
+		mem->zone = zone;
 	mem_hotplug_done();
+
 	return ret;
 }
 
@@ -295,26 +276,12 @@ static int memory_block_offline(struct memory_block *mem)
 		nr_vmemmap_pages = mem->altmap->free;
 
 	mem_hotplug_begin();
-	if (nr_vmemmap_pages)
-		adjust_present_page_count(pfn_to_page(start_pfn), mem->group,
-					  -nr_vmemmap_pages);
-
-	ret = offline_pages(start_pfn + nr_vmemmap_pages,
-			    nr_pages - nr_vmemmap_pages, mem->zone, mem->group);
-	if (ret) {
-		/* offline_pages() failed. Account back. */
-		if (nr_vmemmap_pages)
-			adjust_present_page_count(pfn_to_page(start_pfn),
-						  mem->group, nr_vmemmap_pages);
-		goto out;
-	}
-
-	if (nr_vmemmap_pages)
-		mhp_deinit_memmap_on_memory(start_pfn, nr_vmemmap_pages);
-
-	mem->zone = NULL;
-out:
+	ret = offline_memory_block_pages(start_pfn, nr_pages, nr_vmemmap_pages,
+					 mem->zone, mem->group);
+	if (!ret)
+		mem->zone = NULL;
 	mem_hotplug_done();
+
 	return ret;
 }
 
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index f2f16cdd73ee..1f8d5edd820d 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -106,11 +106,9 @@ extern void adjust_present_page_count(struct page *page,
 				      struct memory_group *group,
 				      long nr_pages);
 /* VM interface that may be used by firmware interface */
-extern int mhp_init_memmap_on_memory(unsigned long pfn, unsigned long nr_pages,
-				     struct zone *zone);
-extern void mhp_deinit_memmap_on_memory(unsigned long pfn, unsigned long nr_pages);
-extern int online_pages(unsigned long pfn, unsigned long nr_pages,
-			struct zone *zone, struct memory_group *group);
+extern int online_memory_block_pages(unsigned long start_pfn,
+		unsigned long nr_pages, unsigned long nr_vmemmap_pages,
+		struct zone *zone, struct memory_group *group);
 extern unsigned long __offline_isolated_pages(unsigned long start_pfn,
 		unsigned long end_pfn);
 
@@ -261,8 +259,9 @@ static inline void pgdat_resize_init(struct pglist_data *pgdat) {}
 #ifdef CONFIG_MEMORY_HOTREMOVE
 
 extern void try_offline_node(int nid);
-extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
-			 struct zone *zone, struct memory_group *group);
+extern int offline_memory_block_pages(unsigned long start_pfn,
+		unsigned long nr_pages, unsigned long nr_vmemmap_pages,
+		struct zone *zone, struct memory_group *group);
 extern int remove_memory(u64 start, u64 size);
 extern void __remove_memory(u64 start, u64 size);
 extern int offline_and_remove_memory(u64 start, u64 size);
@@ -270,8 +269,9 @@ extern int offline_and_remove_memory(u64 start, u64 size);
 #else
 static inline void try_offline_node(int nid) {}
 
-static inline int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
-				struct zone *zone, struct memory_group *group)
+static inline int offline_memory_block_pages(unsigned long start_pfn,
+		unsigned long nr_pages, unsigned long nr_vmemmap_pages,
+		struct zone *zone, struct memory_group *group)
 {
 	return -EINVAL;
 }
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index c8f492b5daf0..8793a83702c5 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1085,7 +1085,7 @@ void adjust_present_page_count(struct page *page, struct memory_group *group,
 		group->present_kernel_pages += nr_pages;
 }
 
-int mhp_init_memmap_on_memory(unsigned long pfn, unsigned long nr_pages,
+static int mhp_init_memmap_on_memory(unsigned long pfn, unsigned long nr_pages,
 			      struct zone *zone)
 {
 	unsigned long end_pfn = pfn + nr_pages;
@@ -1116,7 +1116,7 @@ int mhp_init_memmap_on_memory(unsigned long pfn, unsigned long nr_pages,
 	return ret;
 }
 
-void mhp_deinit_memmap_on_memory(unsigned long pfn, unsigned long nr_pages)
+static void mhp_deinit_memmap_on_memory(unsigned long pfn, unsigned long nr_pages)
 {
 	unsigned long end_pfn = pfn + nr_pages;
 
@@ -1139,7 +1139,7 @@ void mhp_deinit_memmap_on_memory(unsigned long pfn, unsigned long nr_pages)
 /*
  * Must be called with mem_hotplug_lock in write mode.
  */
-int online_pages(unsigned long pfn, unsigned long nr_pages,
+static int online_pages(unsigned long pfn, unsigned long nr_pages,
 		       struct zone *zone, struct memory_group *group)
 {
 	struct memory_notify mem_arg = {
@@ -1254,6 +1254,37 @@ int online_pages(unsigned long pfn, unsigned long nr_pages,
 	return ret;
 }
 
+int online_memory_block_pages(unsigned long start_pfn,
+		unsigned long nr_pages, unsigned long nr_vmemmap_pages,
+		struct zone *zone, struct memory_group *group)
+{
+	int ret;
+
+	if (nr_vmemmap_pages) {
+		ret = mhp_init_memmap_on_memory(start_pfn, nr_vmemmap_pages, zone);
+		if (ret)
+			return ret;
+	}
+
+	ret = online_pages(start_pfn + nr_vmemmap_pages,
+			   nr_pages - nr_vmemmap_pages, zone, group);
+	if (ret) {
+		if (nr_vmemmap_pages)
+			mhp_deinit_memmap_on_memory(start_pfn, nr_vmemmap_pages);
+		return ret;
+	}
+
+	/*
+	 * Account once onlining succeeded. If the zone was unpopulated, it is
+	 * now already properly populated.
+	 */
+	if (nr_vmemmap_pages)
+		adjust_present_page_count(pfn_to_page(start_pfn), group,
+					  nr_vmemmap_pages);
+
+	return ret;
+}
+
 /* we are OK calling __meminit stuff here - we have CONFIG_MEMORY_HOTPLUG */
 static pg_data_t *hotadd_init_pgdat(int nid)
 {
@@ -1896,7 +1927,7 @@ static int count_system_ram_pages_cb(unsigned long start_pfn,
 /*
  * Must be called with mem_hotplug_lock in write mode.
  */
-int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
+static int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
 			struct zone *zone, struct memory_group *group)
 {
 	unsigned long pfn, managed_pages, system_ram_pages = 0;
@@ -2101,6 +2132,32 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
 	return ret;
 }
 
+int offline_memory_block_pages(unsigned long start_pfn,
+		unsigned long nr_pages, unsigned long nr_vmemmap_pages,
+		struct zone *zone, struct memory_group *group)
+{
+	int ret;
+
+	if (nr_vmemmap_pages)
+		adjust_present_page_count(pfn_to_page(start_pfn), group,
+					  -nr_vmemmap_pages);
+
+	ret = offline_pages(start_pfn + nr_vmemmap_pages,
+			    nr_pages - nr_vmemmap_pages, zone, group);
+	if (ret) {
+		/* offline_pages() failed. Account back. */
+		if (nr_vmemmap_pages)
+			adjust_present_page_count(pfn_to_page(start_pfn),
+						  group, nr_vmemmap_pages);
+		return ret;
+	}
+
+	if (nr_vmemmap_pages)
+		mhp_deinit_memmap_on_memory(start_pfn, nr_vmemmap_pages);
+
+	return ret;
+}
+
 static int check_memblock_offlined_cb(struct memory_block *mem, void *arg)
 {
 	int *nid = arg;
-- 
2.47.1

Hi,

On Tue, Jan 20, 2026 at 10:33:45PM +0800, Tianyou Li wrote:
> Encapsulate the mhp_init_memmap_on_memory() and online_pages() into
> online_memory_block_pages(). Thus we can further optimize the
> set_zone_contiguous() to check the whole memory block range, instead
> of check the zone contiguous in separate range.
> 
> Correspondingly, encapsulate the mhp_deinit_memmap_on_memory() and
> offline_pages() into offline_memory_block_pages().
> 
> Tested-by: Yuan Liu <yuan1.liu@intel.com>
> Reviewed-by: Yuan Liu <yuan1.liu@intel.com>
> Signed-off-by: Tianyou Li <tianyou.li@intel.com>
> ---
>  drivers/base/memory.c          | 53 ++++++---------------------
>  include/linux/memory_hotplug.h | 18 +++++-----
>  mm/memory_hotplug.c            | 65 +++++++++++++++++++++++++++++++---
>  3 files changed, 80 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index 751f248ca4a8..ea4d6fbf34fd 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -246,31 +246,12 @@ static int memory_block_online(struct memory_block *mem)
>  		nr_vmemmap_pages = mem->altmap->free;
>  
>  	mem_hotplug_begin();
> -	if (nr_vmemmap_pages) {
> -		ret = mhp_init_memmap_on_memory(start_pfn, nr_vmemmap_pages, zone);
> -		if (ret)
> -			goto out;
> -	}
> -
> -	ret = online_pages(start_pfn + nr_vmemmap_pages,
> -			   nr_pages - nr_vmemmap_pages, zone, mem->group);
> -	if (ret) {
> -		if (nr_vmemmap_pages)
> -			mhp_deinit_memmap_on_memory(start_pfn, nr_vmemmap_pages);
> -		goto out;
> -	}
> -
> -	/*
> -	 * Account once onlining succeeded. If the zone was unpopulated, it is
> -	 * now already properly populated.
> -	 */
> -	if (nr_vmemmap_pages)
> -		adjust_present_page_count(pfn_to_page(start_pfn), mem->group,
> -					  nr_vmemmap_pages);
> -
> -	mem->zone = zone;
> -out:
> +	ret = online_memory_block_pages(start_pfn, nr_pages, nr_vmemmap_pages,
> +					zone, mem->group);
> +	if (!ret)
> +		mem->zone = zone;

I think we can move most of memory_block_online() to the new function and
pass struct memory_block to it.
I'd suggest 
	
	int mhp_block_online(struct memory_block *block)

and 

	int mhp_block_offline(struct memory_block *block)

Other than that LGTM.

>  	mem_hotplug_done();
> +
>  	return ret;
>  }
>  

-- 
Sincerely yours,
Mike.

On 1/22/2026 7:32 PM, Mike Rapoport wrote:
> Hi,
>
> On Tue, Jan 20, 2026 at 10:33:45PM +0800, Tianyou Li wrote:
>> Encapsulate the mhp_init_memmap_on_memory() and online_pages() into
>> online_memory_block_pages(). Thus we can further optimize the
>> set_zone_contiguous() to check the whole memory block range, instead
>> of check the zone contiguous in separate range.
>>
>> Correspondingly, encapsulate the mhp_deinit_memmap_on_memory() and
>> offline_pages() into offline_memory_block_pages().
>>
>> Tested-by: Yuan Liu <yuan1.liu@intel.com>
>> Reviewed-by: Yuan Liu <yuan1.liu@intel.com>
>> Signed-off-by: Tianyou Li <tianyou.li@intel.com>
>> ---
>>   drivers/base/memory.c          | 53 ++++++---------------------
>>   include/linux/memory_hotplug.h | 18 +++++-----
>>   mm/memory_hotplug.c            | 65 +++++++++++++++++++++++++++++++---
>>   3 files changed, 80 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>> index 751f248ca4a8..ea4d6fbf34fd 100644
>> --- a/drivers/base/memory.c
>> +++ b/drivers/base/memory.c
>> @@ -246,31 +246,12 @@ static int memory_block_online(struct memory_block *mem)
>>   		nr_vmemmap_pages = mem->altmap->free;
>>   
>>   	mem_hotplug_begin();
>> -	if (nr_vmemmap_pages) {
>> -		ret = mhp_init_memmap_on_memory(start_pfn, nr_vmemmap_pages, zone);
>> -		if (ret)
>> -			goto out;
>> -	}
>> -
>> -	ret = online_pages(start_pfn + nr_vmemmap_pages,
>> -			   nr_pages - nr_vmemmap_pages, zone, mem->group);
>> -	if (ret) {
>> -		if (nr_vmemmap_pages)
>> -			mhp_deinit_memmap_on_memory(start_pfn, nr_vmemmap_pages);
>> -		goto out;
>> -	}
>> -
>> -	/*
>> -	 * Account once onlining succeeded. If the zone was unpopulated, it is
>> -	 * now already properly populated.
>> -	 */
>> -	if (nr_vmemmap_pages)
>> -		adjust_present_page_count(pfn_to_page(start_pfn), mem->group,
>> -					  nr_vmemmap_pages);
>> -
>> -	mem->zone = zone;
>> -out:
>> +	ret = online_memory_block_pages(start_pfn, nr_pages, nr_vmemmap_pages,
>> +					zone, mem->group);
>> +	if (!ret)
>> +		mem->zone = zone;
> I think we can move most of memory_block_online() to the new function and
> pass struct memory_block to it.
> I'd suggest
> 	
> 	int mhp_block_online(struct memory_block *block)
>
> and
>
> 	int mhp_block_offline(struct memory_block *block)
>
> Other than that LGTM.


It's doable, if not other comments I can change the code. Would it look 
like moving the functions to mm/memory_hotplug.c, change the name to 
mhp_block_online() and mhp_block_offline(), and change the references 
where the original function invoked in drivers/base/memory.c?


My prior thoughts on this was just break the code as small pieces as 
necessary to handle the pages online part together with zone contiguous 
state update.


>
>>   	mem_hotplug_done();
>> +
>>   	return ret;
>>   }
>>

On Sat, Jan 24, 2026 at 08:30:57PM +0800, Li, Tianyou wrote:
> 
> On 1/22/2026 7:32 PM, Mike Rapoport wrote:

> > > diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> > > index 751f248ca4a8..ea4d6fbf34fd 100644
> > > --- a/drivers/base/memory.c
> > > +++ b/drivers/base/memory.c
> > > @@ -246,31 +246,12 @@ static int memory_block_online(struct memory_block *mem)
> > >   		nr_vmemmap_pages = mem->altmap->free;
> > >   	mem_hotplug_begin();
> > > -	if (nr_vmemmap_pages) {
> > > -		ret = mhp_init_memmap_on_memory(start_pfn, nr_vmemmap_pages, zone);
> > > -		if (ret)
> > > -			goto out;
> > > -	}
> > > -
> > > -	ret = online_pages(start_pfn + nr_vmemmap_pages,
> > > -			   nr_pages - nr_vmemmap_pages, zone, mem->group);
> > > -	if (ret) {
> > > -		if (nr_vmemmap_pages)
> > > -			mhp_deinit_memmap_on_memory(start_pfn, nr_vmemmap_pages);
> > > -		goto out;
> > > -	}
> > > -
> > > -	/*
> > > -	 * Account once onlining succeeded. If the zone was unpopulated, it is
> > > -	 * now already properly populated.
> > > -	 */
> > > -	if (nr_vmemmap_pages)
> > > -		adjust_present_page_count(pfn_to_page(start_pfn), mem->group,
> > > -					  nr_vmemmap_pages);
> > > -
> > > -	mem->zone = zone;
> > > -out:
> > > +	ret = online_memory_block_pages(start_pfn, nr_pages, nr_vmemmap_pages,
> > > +					zone, mem->group);
> > > +	if (!ret)
> > > +		mem->zone = zone;
> > I think we can move most of memory_block_online() to the new function and
> > pass struct memory_block to it.
> > I'd suggest
> > 	
> > 	int mhp_block_online(struct memory_block *block)
> > 
> > and
> > 
> > 	int mhp_block_offline(struct memory_block *block)
> > 
> > Other than that LGTM.
> 
> 
> It's doable, if not other comments I can change the code. Would it look like
> moving the functions to mm/memory_hotplug.c, change the name to
> mhp_block_online() and mhp_block_offline(), and change the references where
> the original function invoked in drivers/base/memory.c?

Yeah, that's what I thought about. 

Even more broadly, I think the functionality in drivers/base/memory.c
belongs to mm/ much more than to drivers/ but that's surely out of scope
for these patches.
 
> My prior thoughts on this was just break the code as small pieces as
> necessary to handle the pages online part together with zone contiguous
> state update.

IMHO moving the entire function is cleaner, let's hear what David and Oscar
think.

-- 
Sincerely yours,
Mike.

On 1/27/2026 2:58 PM, Mike Rapoport wrote:
> On Sat, Jan 24, 2026 at 08:30:57PM +0800, Li, Tianyou wrote:
>> On 1/22/2026 7:32 PM, Mike Rapoport wrote:
>>>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>>>> index 751f248ca4a8..ea4d6fbf34fd 100644
>>>> --- a/drivers/base/memory.c
>>>> +++ b/drivers/base/memory.c
>>>> @@ -246,31 +246,12 @@ static int memory_block_online(struct memory_block *mem)
>>>>    		nr_vmemmap_pages = mem->altmap->free;
>>>>    	mem_hotplug_begin();
>>>> -	if (nr_vmemmap_pages) {
>>>> -		ret = mhp_init_memmap_on_memory(start_pfn, nr_vmemmap_pages, zone);
>>>> -		if (ret)
>>>> -			goto out;
>>>> -	}
>>>> -
>>>> -	ret = online_pages(start_pfn + nr_vmemmap_pages,
>>>> -			   nr_pages - nr_vmemmap_pages, zone, mem->group);
>>>> -	if (ret) {
>>>> -		if (nr_vmemmap_pages)
>>>> -			mhp_deinit_memmap_on_memory(start_pfn, nr_vmemmap_pages);
>>>> -		goto out;
>>>> -	}
>>>> -
>>>> -	/*
>>>> -	 * Account once onlining succeeded. If the zone was unpopulated, it is
>>>> -	 * now already properly populated.
>>>> -	 */
>>>> -	if (nr_vmemmap_pages)
>>>> -		adjust_present_page_count(pfn_to_page(start_pfn), mem->group,
>>>> -					  nr_vmemmap_pages);
>>>> -
>>>> -	mem->zone = zone;
>>>> -out:
>>>> +	ret = online_memory_block_pages(start_pfn, nr_pages, nr_vmemmap_pages,
>>>> +					zone, mem->group);
>>>> +	if (!ret)
>>>> +		mem->zone = zone;
>>> I think we can move most of memory_block_online() to the new function and
>>> pass struct memory_block to it.
>>> I'd suggest
>>> 	
>>> 	int mhp_block_online(struct memory_block *block)
>>>
>>> and
>>>
>>> 	int mhp_block_offline(struct memory_block *block)
>>>
>>> Other than that LGTM.
>>
>> It's doable, if not other comments I can change the code. Would it look like
>> moving the functions to mm/memory_hotplug.c, change the name to
>> mhp_block_online() and mhp_block_offline(), and change the references where
>> the original function invoked in drivers/base/memory.c?
> Yeah, that's what I thought about.
>
> Even more broadly, I think the functionality in drivers/base/memory.c
> belongs to mm/ much more than to drivers/ but that's surely out of scope
> for these patches.
>   


Thanks Mike for the confirmation.


>> My prior thoughts on this was just break the code as small pieces as
>> necessary to handle the pages online part together with zone contiguous
>> state update.
> IMHO moving the entire function is cleaner, let's hear what David and Oscar
> think.


Sure. Actually the patch has been ready with the mhp_block_online() and 
mhp_block_offline() moved to memory_hotplug.c, the 
online_memory_block_pages() and offline_memory_block_pages() remains but 
as static function.


Once get feedback from David and Oscar, I can change them accordingly.