linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: Tianyou Li <tianyou.li@intel.com>
Cc: David Hildenbrand <david@redhat.com>,
	Oscar Salvador <osalvador@suse.de>,
	Wei Yang <richard.weiyang@gmail.com>,
	linux-mm@kvack.org, Yong Hu <yong.hu@intel.com>,
	Nanhai Zou <nanhai.zou@intel.com>, Yuan Liu <yuan1.liu@intel.com>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Qiuxu Zhuo <qiuxu.zhuo@intel.com>,
	Yu C Chen <yu.c.chen@intel.com>, Pan Deng <pan.deng@intel.com>,
	Chen Zhang <zhangchen.kidd@jd.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] mm/memory hotplug/unplug: Optimize zone->contiguous update when move pfn range
Date: Thu, 20 Nov 2025 14:00:07 +0200	[thread overview]
Message-ID: <aR8Cx_K2N8DPlwkP@kernel.org> (raw)
In-Reply-To: <20251119140657.3845818-1-tianyou.li@intel.com>

Hi,

Please start a new thread when sending a new version of a patch next time.
And as Wei mentioned, wait a bit for the discussion on vN to settle before
sending vN+1.

On Wed, Nov 19, 2025 at 10:06:57PM +0800, Tianyou Li wrote:
> When invoke move_pfn_range_to_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 |
> |                +------+---------------+--------------+----------------+
> | Memory Hotplug | 256G |      10s      |      2s      |       80%      |
> |                +------+---------------+--------------+----------------+
> |                | 512G |      33s      |      6s      |       81%      |
> +----------------+------+---------------+--------------+----------------+
> 
> [1] Qemu commands to hotplug 512G memory for a VM:
>     object_add memory-backend-ram,id=hotmem0,size=512G,share=on
>     device_add virtio-mem-pci,id=vmem1,memdev=hotmem0,bus=port1
>     qom-set vmem1 requested-size 512G
> 
> [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>
> ---
>  mm/memory_hotplug.c | 51 ++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 0be83039c3b5..aed1827a2778 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -723,6 +723,51 @@ static void __meminit resize_pgdat_range(struct pglist_data *pgdat, unsigned lon
>  
>  }
>  
> +static bool __meminit check_zone_contiguous_fast(struct zone *zone,
> +			unsigned long start_pfn, unsigned long nr_pages)
> +{
> +	const unsigned long end_pfn = start_pfn + nr_pages;
> +
> +	/*
> +	 * Given the moved pfn range's contiguous property is always true,
> +	 * under the conditional of empty zone, the contiguous property should
> +	 * be true.
> +	 */
> +	if (zone_is_empty(zone)) {
> +		zone->contiguous = true;

I don't think it's safe to set zone->contiguous until the end of
move_pfn_range_to_zone(). See commit feee6b298916 ("mm/memory_hotplug:
shrink zones when offlining memory").

check_zone_contiguous_fast() should only check if the zone remains
contiguous after hotplug or it's certainly discontinuous, but should not
set zone->contiguous. It still must be cleared before resizing the zone and
set after the initialization of the memory map.

> +		return true;
> +	}
> +
> +	/*
> +	 * If the moved pfn range does not intersect with the original zone span,
> +	 * the contiguous property is surely false.
> +	 */
> +	if (end_pfn < zone->zone_start_pfn || start_pfn > zone_end_pfn(zone)) {
> +		zone->contiguous = false;
> +		return true;
> +	}
> +
> +	/*
> +	 * If the moved pfn range is adjacent to the original zone span, given
> +	 * the moved pfn range's contiguous property is always true, the zone's
> +	 * contiguous property inherited from the original value.
> +	 */
> +	if (end_pfn == zone->zone_start_pfn || start_pfn == zone_end_pfn(zone))
> +		return true;
> +
> +	/*
> +	 * If the original zone's hole larger than the moved pages in the range,
> +	 * the contiguous property is surely false.
> +	 */
> +	if (nr_pages < (zone->spanned_pages - zone->present_pages)) {
> +		zone->contiguous = false;
> +		return true;
> +	}
> +
> +	clear_zone_contiguous(zone);
> +	return false;
> +}
> +
>  #ifdef CONFIG_ZONE_DEVICE
>  static void section_taint_zone_device(unsigned long pfn)
>  {
> @@ -752,8 +797,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;
> -
> -	clear_zone_contiguous(zone);
> +	const bool fast_path = check_zone_contiguous_fast(zone, start_pfn, nr_pages);
>
>  	if (zone_is_empty(zone))
>  		init_currently_empty_zone(zone, start_pfn, nr_pages);
> @@ -783,7 +827,8 @@ void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  			 MEMINIT_HOTPLUG, altmap, migratetype,
>  			 isolate_pageblock);
>  
> -	set_zone_contiguous(zone);
> +	if (!fast_path)
> +		set_zone_contiguous(zone);
>  }
>  
>  struct auto_movable_stats {
> -- 
> 2.47.1
> 

-- 
Sincerely yours,
Mike.


  reply	other threads:[~2025-11-20 12:00 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-17  3:30 [PATCH] " Tianyou Li
2025-11-17  2:38 ` Li, Tianyou
2025-11-17 11:57 ` David Hildenbrand (Red Hat)
2025-11-18  9:07   ` Li, Tianyou
2025-11-18  5:13 ` Mike Rapoport
2025-11-18  9:28   ` Li, Tianyou
2025-11-18  9:35   ` Li, Tianyou
2025-11-18 10:31     ` Li, Tianyou
2025-11-19  4:07   ` [PATCH v2] " Tianyou Li
2025-11-19  3:13     ` Li, Tianyou
2025-11-28 11:49       ` David Hildenbrand (Red Hat)
2025-11-28 13:33         ` Li, Tianyou
2025-11-19 11:42     ` Wei Yang
2025-11-19 12:41       ` Li, Tianyou
2025-11-19 12:44         ` Wei Yang
2025-11-19 13:16           ` Li, Tianyou
2025-11-19 14:06       ` [PATCH v3] " Tianyou Li
2025-11-20 12:00         ` Mike Rapoport [this message]
2025-11-20 14:21           ` Li, Tianyou
2025-11-28 12:01         ` David Hildenbrand (Red Hat)
2025-11-28 15:17           ` Li, Tianyou
2025-11-28 16:04             ` David Hildenbrand (Red Hat)
2025-12-01 12:28               ` Li, Tianyou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aR8Cx_K2N8DPlwkP@kernel.org \
    --to=rppt@kernel.org \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nanhai.zou@intel.com \
    --cc=osalvador@suse.de \
    --cc=pan.deng@intel.com \
    --cc=qiuxu.zhuo@intel.com \
    --cc=richard.weiyang@gmail.com \
    --cc=tianyou.li@intel.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=yong.hu@intel.com \
    --cc=yu.c.chen@intel.com \
    --cc=yuan1.liu@intel.com \
    --cc=zhangchen.kidd@jd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox