Re: [PATCH v9 2/2] mm/memory hotplug/unplug: Optimize zone->contiguous update when changes pfn range

["Li, Tianyou" <tianyou.li@intel.com>]

Hi David, Mike & Other maintainers,

Just a quick update: Yuan and I created a new patch based on David and 
Mike's code, updated the documentation as well, performed the 
performance test, double check all the references etc. We observed a 
slightly performance regression, not sure it was caused by the 
environment configuration or we triggered some unexpected behavior. We 
might need a bit more time to investigate. Will send out the patch soon 
once we have confirmed data for the performance regression. Thanks.


Regards,

Tianyou


On 2/24/2026 3:15 PM, Li, Tianyou wrote:
>
> On 2/11/2026 8:19 PM, David Hildenbrand (Arm) wrote:
>>>>        *
>>>> +     * online_pages is pages within the zone that have an online 
>>>> memmap.
>>>> +     * online_pages include present pages and memory holes that 
>>>> have a
>>>> +     * memmap. When spanned_pages == online_pages, pfn_to_page() 
>>>> can be
>>>> +     * performed without further checks on any pfn within the zone 
>>>> span.
>>>
>>> Maybe pages_with_memmap? It would stand off from managed, spanned and
>>> present, but it's clearer than online IMHO.
>>
>> offline pages also have a memmap, but that should not be touched as 
>> it might contain garbage. So it's a bit more tricky :)
>>
>>>
>>>> +     *
>>>>        * So present_pages may be used by memory hotplug or memory 
>>>> power
>>>>        * management logic to figure out unmanaged pages by checking
>>>>        * (present_pages - managed_pages). And managed_pages should 
>>>> be used
>>>> @@ -967,6 +972,7 @@ struct zone {
>>>>       atomic_long_t        managed_pages;
>>>>       unsigned long        spanned_pages;
>>>>       unsigned long        present_pages;
>>>> +    unsigned long        online_pages;
>>>>   #if defined(CONFIG_MEMORY_HOTPLUG)
>>>>       unsigned long        present_early_pages;
>>>>   #endif
>>>> @@ -1051,8 +1057,6 @@ struct zone {
>>>>       bool            compact_blockskip_flush;
>>>>   #endif
>>>> -    bool            contiguous;
>>>> -
>>>>       CACHELINE_PADDING(_pad3_);
>>>>       /* Zone statistics */
>>>>       atomic_long_t        vm_stat[NR_VM_ZONE_STAT_ITEMS];
>>>> @@ -1124,6 +1128,23 @@ static inline bool zone_spans_pfn(const 
>>>> struct zone *zone, unsigned long pfn)
>>>>       return zone->zone_start_pfn <= pfn && pfn < zone_end_pfn(zone);
>>>>   }
>>>> +/**
>>>> + * zone_is_contiguous - test whether a zone is contiguous
>>>> + * @zone: the zone to test.
>>>> + *
>>>> + * In a contiguous zone, it is valid to call pfn_to_page() on any 
>>>> pfn in the
>>>> + * spanned zone without requiting pfn_valid() or 
>>>> pfn_to_online_page() checks.
>>>> + *
>>>> + * Returns: true if contiguous, otherwise false.
>>>> + */
>>>> +static inline bool zone_is_contiguous(const struct zone *zone)
>>>> +{
>>>> +    return READ_ONCE(zone->spanned_pages) == 
>>>> READ_ONCE(zone->online_pages);
>>>> +}
>>>> +
>>>>   static inline bool zone_is_initialized(const struct zone *zone)
>>>>   {
>>>>       return zone->initialized;
>>>> diff --git a/mm/internal.h b/mm/internal.h
>>>> index f35dbcf99a86..6062f9b8ee62 100644
>>>> --- a/mm/internal.h
>>>> +++ b/mm/internal.h
>>>> @@ -716,21 +716,15 @@ extern struct page 
>>>> *__pageblock_pfn_to_page(unsigned long start_pfn,
>>>>   static inline struct page *pageblock_pfn_to_page(unsigned long 
>>>> start_pfn,
>>>>                   unsigned long end_pfn, struct zone *zone)
>>>>   {
>>>> -    if (zone->contiguous)
>>>> +    if (zone_is_contiguous(zone))
>>>>           return pfn_to_page(start_pfn);
>>>>       return __pageblock_pfn_to_page(start_pfn, end_pfn, zone);
>>>>   }
>>>> -void set_zone_contiguous(struct zone *zone);
>>>>   bool pfn_range_intersects_zones(int nid, unsigned long start_pfn,
>>>>                  unsigned long nr_pages);
>>>> -static inline void clear_zone_contiguous(struct zone *zone)
>>>> -{
>>>> -    zone->contiguous = false;
>>>> -}
>>>> -
>>>>   extern int __isolate_free_page(struct page *page, unsigned int 
>>>> order);
>>>>   extern void __putback_isolated_page(struct page *page, unsigned 
>>>> int order,
>>>>                       int mt);
>>>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>>> index a63ec679d861..76496c1039a9 100644
>>>> --- a/mm/memory_hotplug.c
>>>> +++ b/mm/memory_hotplug.c
>>>> @@ -492,11 +492,11 @@ static void shrink_zone_span(struct zone 
>>>> *zone, unsigned long start_pfn,
>>>>           pfn = find_smallest_section_pfn(nid, zone, end_pfn,
>>>>                           zone_end_pfn(zone));
>>>>           if (pfn) {
>>>> -            zone->spanned_pages = zone_end_pfn(zone) - pfn;
>>>> +            WRITE_ONCE(zone->spanned_pages, zone_end_pfn(zone) - 
>>>> pfn);
>>>>               zone->zone_start_pfn = pfn;
>>>>           } else {
>>>>               zone->zone_start_pfn = 0;
>>>> -            zone->spanned_pages = 0;
>>>> +            WRITE_ONCE(zone->spanned_pages, 0);
>>>>           }
>>>>       } else if (zone_end_pfn(zone) == end_pfn) {
>>>>           /*
>>>> @@ -508,10 +508,10 @@ static void shrink_zone_span(struct zone 
>>>> *zone, unsigned long start_pfn,
>>>>           pfn = find_biggest_section_pfn(nid, zone, 
>>>> zone->zone_start_pfn,
>>>>                              start_pfn);
>>>>           if (pfn)
>>>> -            zone->spanned_pages = pfn - zone->zone_start_pfn + 1;
>>>> +            WRITE_ONCE(zone->spanned_pages, pfn - 
>>>> zone->zone_start_pfn + 1);
>>>>           else {
>>>>               zone->zone_start_pfn = 0;
>>>> -            zone->spanned_pages = 0;
>>>> +            WRITE_ONCE(zone->spanned_pages, 0);
>>>>           }
>>>>       }
>>>>   }
>>>> @@ -565,18 +565,13 @@ void remove_pfn_range_from_zone(struct zone 
>>>> *zone,
>>>>       /*
>>>>        * Zone shrinking code cannot properly deal with ZONE_DEVICE. So
>>>> -     * we will not try to shrink the zones - which is okay as
>>>> -     * set_zone_contiguous() cannot deal with ZONE_DEVICE either way.
>>>> +     * we will not try to shrink the zones.
>>>>        */
>>>>       if (zone_is_zone_device(zone))
>>>>           return;
>>>> -    clear_zone_contiguous(zone);
>>>> -
>>>>       shrink_zone_span(zone, start_pfn, start_pfn + nr_pages);
>>>>       update_pgdat_span(pgdat);
>>>> -
>>>> -    set_zone_contiguous(zone);
>>>>   }
>>>>   /**
>>>> @@ -753,8 +748,6 @@ 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;
>>>> -    clear_zone_contiguous(zone);
>>>> -
>>>>       if (zone_is_empty(zone))
>>>>           init_currently_empty_zone(zone, start_pfn, nr_pages);
>>>>       resize_zone_range(zone, start_pfn, nr_pages);
>>>> @@ -782,8 +775,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);
>>>>   }
>>>>   struct auto_movable_stats {
>>>> @@ -1079,6 +1070,7 @@ void adjust_present_page_count(struct page 
>>>> *page, struct memory_group *group,
>>>>       if (early_section(__pfn_to_section(page_to_pfn(page))))
>>>>           zone->present_early_pages += nr_pages;
>>>>       zone->present_pages += nr_pages;
>>>> +    WRITE_ONCE(zone->online_pages, zone->online_pages + nr_pages);
>>>>       zone->zone_pgdat->node_present_pages += nr_pages;
>>>>       if (group && movable)
>>>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>>>> index 2a809cd8e7fa..e33caa6fb6fc 100644
>>>> --- a/mm/mm_init.c
>>>> +++ b/mm/mm_init.c
>>>> @@ -2263,9 +2263,10 @@ void __init init_cma_pageblock(struct page 
>>>> *page)
>>>>   }
>>>>   #endif
>>>> -void set_zone_contiguous(struct zone *zone)
>>>> +static void calc_online_pages(struct zone *zone)
>>>>   {
>>>>       unsigned long block_start_pfn = zone->zone_start_pfn;
>>>> +    unsigned long online_pages = 0;
>>>>       unsigned long block_end_pfn;
>>>>       block_end_pfn = pageblock_end_pfn(block_start_pfn);
>>>> @@ -2277,12 +2278,11 @@ void set_zone_contiguous(struct zone *zone)
>>>>           if (!__pageblock_pfn_to_page(block_start_pfn,
>>>>                            block_end_pfn, zone))
>>>> -            return;
>>>> +            continue;
>>>>           cond_resched();
>>>> +        online_pages += block_end_pfn - block_start_pfn;
>>>
>>> I think we can completely get rid of this with something like this 
>>> untested
>>> patch to calculate zone->online_pages for coldplug:
>>>
>>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>>> index e33caa6fb6fc..ff2f75e7b49f 100644
>>> --- a/mm/mm_init.c
>>> +++ b/mm/mm_init.c
>>> @@ -845,9 +845,9 @@ overlap_memmap_init(unsigned long zone, unsigned 
>>> long *pfn)
>>>    *   zone/node above the hole except for the trailing pages in the 
>>> last
>>>    *   section that will be appended to the zone/node below.
>>>    */
>>> -static void __init init_unavailable_range(unsigned long spfn,
>>> -                      unsigned long epfn,
>>> -                      int zone, int node)
>>> +static u64 __init init_unavailable_range(unsigned long spfn,
>>> +                     unsigned long epfn,
>>> +                     int zone, int node)
>>>   {
>>>       unsigned long pfn;
>>>       u64 pgcnt = 0;
>>> @@ -861,6 +861,8 @@ static void __init 
>>> init_unavailable_range(unsigned long spfn,
>>>       if (pgcnt)
>>>           pr_info("On node %d, zone %s: %lld pages in unavailable 
>>> ranges\n",
>>>               node, zone_names[zone], pgcnt);
>>> +
>>> +    return pgcnt;
>>>   }
>>>     /*
>>> @@ -959,9 +961,10 @@ static void __init 
>>> memmap_init_zone_range(struct zone *zone,
>>>       memmap_init_range(end_pfn - start_pfn, nid, zone_id, start_pfn,
>>>                 zone_end_pfn, MEMINIT_EARLY, NULL, MIGRATE_MOVABLE,
>>>                 false);
>>> +    zone->online_pages += (end_pfn - start_pfn);
>>>         if (*hole_pfn < start_pfn)
>>> -        init_unavailable_range(*hole_pfn, start_pfn, zone_id, nid);
>>> +        zone->online_pages += init_unavailable_range(*hole_pfn, 
>>> start_pfn, zone_id, nid);
>>>         *hole_pfn = end_pfn;
>>>   }
>>>
>>
>> Looking at set_zone_contiguous(), __pageblock_pfn_to_page() takes 
>> care of a weird case where the end of a zone falls into the middle of 
>> a pageblock.
>>
>> I am not even sure if that is possible, but we could handle that 
>> easily in pageblock_pfn_to_page() by checking the requested range 
>> against the zone spanned range.
>>
>> Then the semantics "zone->online_pages" would be less weird and more 
>> closely resemble "pages with online memmap".
>>
>> init_unavailable_range() might indeed do the trick!
>>
>> @Tianyou, can you explore that direction? I know, your PTO is coming up.
>>
>
> Hi David,
>
> Sorry for the late response, just come back from CNY holiday. I am 
> willing to explore the new direction. Will sync with Yuan in a day or 
> two to confirm the overall implementation and testing plan. Thanks in 
> advance for your time to review the new patch set once ready.
>
> Regards,
>
> Tianyou
>
>
>
>