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

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

Thank you all for your time and efforts. We have submitted the patch v6, 
tested through the last weekend. Any comments or suggestions are 
welcomed. Appreciated.


Regards,

Tianyou


On 12/12/2025 1:27 PM, Li, Tianyou wrote:
>
> On 12/11/2025 1:07 PM, Oscar Salvador wrote:
>> On Mon, Dec 08, 2025 at 11:25:43PM +0800, Tianyou Li wrote:
>>> When invoke move_pfn_range_to_zone or remove_pfn_range_from_zone, it 
>>> will
>>> update the zone->contiguous by checking the new zone's pfn range 
>>> from the
>>> beginning to the end, regardless the previous state of the old zone. 
>>> When
>>> the zone's pfn range is large, the cost of traversing the pfn range to
>>> update the zone->contiguous could be significant.
>>>
>>> Add fast paths to quickly detect cases where zone is definitely not
>>> contiguous without scanning the new zone. The cases are: when the 
>>> new range
>>> did not overlap with previous range, the contiguous should be false; 
>>> if the
>>> new range adjacent with the previous range, just need to check the new
>>> range; if the new added pages could not fill the hole of previous 
>>> zone, the
>>> contiguous should be false.
>>>
>>> The following test cases of memory hotplug for a VM [1], tested in the
>>> environment [2], show that this optimization can significantly 
>>> reduce the
>>> memory hotplug time [3].
>>>
>>> +----------------+------+---------------+--------------+----------------+ 
>>>
>>> |                | Size | Time (before) | Time (after) | Time 
>>> Reduction |
>>> | +------+---------------+--------------+----------------+
>>> | Plug Memory    | 256G |      10s      |      2s      | 80%      |
>>> | +------+---------------+--------------+----------------+
>>> |                | 512G |      33s      |      6s      | 81%      |
>>> +----------------+------+---------------+--------------+----------------+ 
>>>
>>>
>>> +----------------+------+---------------+--------------+----------------+ 
>>>
>>> |                | Size | Time (before) | Time (after) | Time 
>>> Reduction |
>>> | +------+---------------+--------------+----------------+
>>> | Unplug Memory  | 256G |      10s      |      2s      | 80%      |
>>> | +------+---------------+--------------+----------------+
>>> |                | 512G |      34s      |      6s      | 82%      |
>>> +----------------+------+---------------+--------------+----------------+ 
>>>
>>>
>>> [1] Qemu commands to hotplug 256G/512G memory for a VM:
>>>      object_add memory-backend-ram,id=hotmem0,size=256G/512G,share=on
>>>      device_add virtio-mem-pci,id=vmem1,memdev=hotmem0,bus=port1
>>>      qom-set vmem1 requested-size 256G/512G (Plug Memory)
>>>      qom-set vmem1 requested-size 0G (Unplug Memory)
>>>
>>> [2] Hardware     : Intel Icelake server
>>>      Guest Kernel : v6.18-rc2
>>>      Qemu         : v9.0.0
>>>
>>>      Launch VM    :
>>>      qemu-system-x86_64 -accel kvm -cpu host \
>>>      -drive file=./Centos10_cloud.qcow2,format=qcow2,if=virtio \
>>>      -drive file=./seed.img,format=raw,if=virtio \
>>>      -smp 3,cores=3,threads=1,sockets=1,maxcpus=3 \
>>>      -m 2G,slots=10,maxmem=2052472M \
>>>      -device 
>>> pcie-root-port,id=port1,bus=pcie.0,slot=1,multifunction=on \
>>>      -device pcie-root-port,id=port2,bus=pcie.0,slot=2 \
>>>      -nographic -machine q35 \
>>>      -nic user,hostfwd=tcp::3000-:22
>>>
>>>      Guest kernel auto-onlines newly added memory blocks:
>>>      echo online > /sys/devices/system/memory/auto_online_blocks
>>>
>>> [3] The time from typing the QEMU commands in [1] to when the output of
>>>      'grep MemTotal /proc/meminfo' on Guest reflects that all 
>>> hotplugged
>>>      memory is recognized.
>>>
>>> Reported-by: Nanhai Zou <nanhai.zou@intel.com>
>>> Reported-by: Chen Zhang <zhangchen.kidd@jd.com>
>>> Tested-by: Yuan Liu <yuan1.liu@intel.com>
>>> Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>
>>> Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
>>> Reviewed-by: Yu C Chen <yu.c.chen@intel.com>
>>> Reviewed-by: Pan Deng <pan.deng@intel.com>
>>> Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
>>> Reviewed-by: Yuan Liu <yuan1.liu@intel.com>
>>> Signed-off-by: Tianyou Li <tianyou.li@intel.com>
>> Overall this looks good to me, thanks Tianyou Li for working on this.
>> Just some minor comments below:
>
>
> Very appreciated Oscar for your time to review the patch! Yuan and I 
> will work on the patch v6 to address all your comments/suggestions.
>
> We will test the patch v6 across the weekend. If any other comments 
> during those days, we will need more days probably. Thanks.
>
>
>>> ---
>>>   mm/internal.h       |  8 +++++-
>>>   mm/memory_hotplug.c | 64 
>>> ++++++++++++++++++++++++++++++++++++++++++---
>>>   mm/mm_init.c        | 13 +++++++--
>>>   3 files changed, 79 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/mm/internal.h b/mm/internal.h
>>> index 1561fc2ff5b8..1b5bba6526d4 100644
>>> --- a/mm/internal.h
>>> +++ b/mm/internal.h
>>> @@ -730,7 +730,13 @@ static inline struct page 
>>> *pageblock_pfn_to_page(unsigned long start_pfn,
>>>       return __pageblock_pfn_to_page(start_pfn, end_pfn, zone);
>>>   }
>>>   -void set_zone_contiguous(struct zone *zone);
>>> +enum zone_contig_state {
>>> +    ZONE_CONTIG_YES,
>>> +    ZONE_CONTIG_NO,
>>> +    ZONE_CONTIG_MAYBE,
>>> +};
>>> +
>>> +void set_zone_contiguous(struct zone *zone, enum zone_contig_state 
>>> state);
>>>   bool pfn_range_intersects_zones(int nid, unsigned long start_pfn,
>>>                  unsigned long nr_pages);
>>>   diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>> index 0be83039c3b5..d711f6e2c87f 100644
>>> --- a/mm/memory_hotplug.c
>>> +++ b/mm/memory_hotplug.c
>>> @@ -544,6 +544,28 @@ static void update_pgdat_span(struct 
>>> pglist_data *pgdat)
>>>       pgdat->node_spanned_pages = node_end_pfn - node_start_pfn;
>>>   }
>>>   +static enum zone_contig_state __meminit 
>>> zone_contig_state_after_shrinking(
>>> +        struct zone *zone, unsigned long start_pfn, unsigned long 
>>> nr_pages)
>> Why do we need the __meminit? These functions are only used from 
>> memory-hotplug
>> code so we should not need it?
>
>
> Got it. Will remove the __meminit in patch v6.
>
>
>>> +{
>>> +    const unsigned long end_pfn = start_pfn + nr_pages;
>>> +
>>> +    /*
>>> +     * If the removed pfn range inside the original zone span, the 
>>> contiguous
>>> +     * property is surely false.
>>> +     */
>>> +    if (start_pfn > zone->zone_start_pfn && end_pfn < 
>>> zone_end_pfn(zone))
>>> +        return ZONE_CONTIG_NO;
>>> +
>>> +    /* If the removed pfn range is at the beginning or end of the
>>> +     * original zone span, the contiguous property is preserved when
>>> +     * the original zone is contiguous.
>>> +     */
>>> +    if (start_pfn == zone->zone_start_pfn || end_pfn == 
>>> zone_end_pfn(zone))
>>> +        return zone->contiguous ? ZONE_CONTIG_YES : ZONE_CONTIG_MAYBE;
>>> +
>>> +    return ZONE_CONTIG_MAYBE;
>>> +}
>>> +
>>>   void remove_pfn_range_from_zone(struct zone *zone,
>>>                         unsigned long start_pfn,
>>>                         unsigned long nr_pages)
>>> @@ -551,6 +573,7 @@ void remove_pfn_range_from_zone(struct zone *zone,
>>>       const unsigned long end_pfn = start_pfn + nr_pages;
>>>       struct pglist_data *pgdat = zone->zone_pgdat;
>>>       unsigned long pfn, cur_nr_pages;
>>> +    enum zone_contig_state contiguous_state = ZONE_CONTIG_MAYBE;
>> I think that new_contiguous_state is clearer, but I do not have a strong
>> opinion here.
>
>
> Will do in the patch v6.
>
>
>>>       /* Poison struct pages because they are now uninitialized 
>>> again. */
>>>       for (pfn = start_pfn; pfn < end_pfn; pfn += cur_nr_pages) {
>>> @@ -571,12 +594,13 @@ void remove_pfn_range_from_zone(struct zone 
>>> *zone,
>>>       if (zone_is_zone_device(zone))
>>>           return;
>>>   +    contiguous_state = zone_contig_state_after_shrinking(zone, 
>>> start_pfn, nr_pages);
>>>       clear_zone_contiguous(zone);
>>>         shrink_zone_span(zone, start_pfn, start_pfn + nr_pages);
>>>       update_pgdat_span(pgdat);
>>>   -    set_zone_contiguous(zone);
>>> +    set_zone_contiguous(zone, contiguous_state);
>>>   }
>> ...
>>> @@ -752,7 +809,8 @@ 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);
>> Same comment from remove_pfn_range_from_zone.
>
>
> Will do in the patch v6.
>
>
>>>       clear_zone_contiguous(zone);
>>>         if (zone_is_empty(zone))
>>> @@ -783,7 +841,7 @@ void move_pfn_range_to_zone(struct zone *zone, 
>>> unsigned long start_pfn,
>>>                MEMINIT_HOTPLUG, altmap, migratetype,
>>>                isolate_pageblock);
>>>   -    set_zone_contiguous(zone);
>>> +    set_zone_contiguous(zone, contiguous_state);
>>>   }
>>>     struct auto_movable_stats {
>>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>>> index 7712d887b696..e296bd9fac9e 100644
>>> --- a/mm/mm_init.c
>>> +++ b/mm/mm_init.c
>>> @@ -2263,11 +2263,19 @@ void __init init_cma_pageblock(struct page 
>>> *page)
>>>   }
>>>   #endif
>>>   -void set_zone_contiguous(struct zone *zone)
>>> +void set_zone_contiguous(struct zone *zone, enum zone_contig_state 
>>> state)
>>>   {
>>>       unsigned long block_start_pfn = zone->zone_start_pfn;
>>>       unsigned long block_end_pfn;
>>>   +    if (state == ZONE_CONTIG_YES) {
>>> +        zone->contiguous = true;
>>> +        return;
>>> +    }
>>> +
>>> +    if (state == ZONE_CONTIG_NO)
>>> +        return;
>>> +
>>>       block_end_pfn = pageblock_end_pfn(block_start_pfn);
>>>       for (; block_start_pfn < zone_end_pfn(zone);
>>>               block_start_pfn = block_end_pfn,
>>> @@ -2283,6 +2291,7 @@ void set_zone_contiguous(struct zone *zone)
>>>         /* We confirm that there is no hole */
>>>       zone->contiguous = true;
>>> +
>> Not needed?
>
>
> My mistake, thanks. Will do in the patch v6.
>
>
> Regards,
>
> Tianyou
>
>
>>>   }
>>
>
>