Re: [PATCH v5 2/2] mm/memory hotplug: fix zone->contiguous always false when hotplug

[Oscar Salvador <osalvador@suse.de>]

On Mon, Dec 08, 2025 at 11:25:44PM +0800, Tianyou Li wrote:
> From: Yuan Liu <yuan1.liu@intel.com>
> 
> Function set_zone_contiguous used __pageblock_pfn_to_page to
> check the whole pageblock is in the same zone. One assumption is
> the memory section must online, otherwise the __pageblock_pfn_to_page
> will return NULL, then the set_zone_contiguous will be false.
> When move_pfn_range_to_zone invoked set_zone_contiguous, since the
> memory section did not online, the return value will always be false.

Then, this means that zone->contiguous was always left false on new
memory-hotplug operations, if it was false before?

I guess we did not notice this before because it is just an optimization
so we always took the long road.

Nice catch.

> To fix this issue, we removed the set_zone_contiguous from the
> move_pfn_range_to_zone, and place it after memory section onlined.
> 
> Function remove_pfn_range_from_zone did not have this issue because
> memory section remains online at the time set_zone_contiguous invoked.
> 
> Reviewed-by: Tianyou Li <tianyou.li@intel.com>
> Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
> Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
> ---
>  mm/memory_hotplug.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index d711f6e2c87f..f548d9180415 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -809,8 +809,7 @@ void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  {
>  	struct pglist_data *pgdat = zone->zone_pgdat;
>  	int nid = pgdat->node_id;
> -	const enum zone_contig_state contiguous_state =
> -		zone_contig_state_after_growing(zone, start_pfn, nr_pages);
> +
>  	clear_zone_contiguous(zone);
>  
>  	if (zone_is_empty(zone))
> @@ -840,8 +839,6 @@ void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  	memmap_init_range(nr_pages, nid, zone_idx(zone), start_pfn, 0,
>  			 MEMINIT_HOTPLUG, altmap, migratetype,
>  			 isolate_pageblock);
> -
> -	set_zone_contiguous(zone, contiguous_state);
>  }
>  
>  struct auto_movable_stats {
> @@ -1150,6 +1147,7 @@ int mhp_init_memmap_on_memory(unsigned long pfn, unsigned long nr_pages,
>  {
>  	unsigned long end_pfn = pfn + nr_pages;
>  	int ret, i;
> +	enum zone_contig_state contiguous_state = ZONE_CONTIG_NO;
>  
>  	ret = kasan_add_zero_shadow(__va(PFN_PHYS(pfn)), PFN_PHYS(nr_pages));
>  	if (ret)
> @@ -1164,6 +1162,10 @@ int mhp_init_memmap_on_memory(unsigned long pfn, unsigned long nr_pages,
>  	if (mhp_off_inaccessible)
>  		page_init_poison(pfn_to_page(pfn), sizeof(struct page) * nr_pages);
>  
> +	if (IS_ALIGNED(end_pfn, PAGES_PER_SECTION))
> +		contiguous_state = zone_contig_state_after_growing(zone, pfn,
> +								   nr_pages);

Uhm, I think this deserves a little comment.
I guess that if we are not allocating memmap pages worth of a full
section, we keep it ZONE_CONTIG_NO.

 

-- 
Oscar Salvador
SUSE Labs