lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3cb317fa-abe0-4946-9f00-da00bade2def@kernel.org>
Date: Sat, 7 Feb 2026 12:00:09 +0100
From: "David Hildenbrand (Arm)" <david@...nel.org>
To: Tianyou Li <tianyou.li@...el.com>, Oscar Salvador <osalvador@...e.de>,
 Mike Rapoport <rppt@...nel.org>, Wei Yang <richard.weiyang@...il.com>,
 Michal Hocko <mhocko@...e.com>
Cc: linux-mm@...ck.org, Yong Hu <yong.hu@...el.com>,
 Nanhai Zou <nanhai.zou@...el.com>, Yuan Liu <yuan1.liu@...el.com>,
 Tim Chen <tim.c.chen@...ux.intel.com>, Qiuxu Zhuo <qiuxu.zhuo@...el.com>,
 Yu C Chen <yu.c.chen@...el.com>, Pan Deng <pan.deng@...el.com>,
 Chen Zhang <zhangchen.kidd@...com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 2/2] mm/memory hotplug/unplug: Optimize
 zone->contiguous update when changes pfn range

On 1/30/26 17:37, 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@...el.com>
> Reported-by: Chen Zhang <zhangchen.kidd@...com>
> Tested-by: Yuan Liu <yuan1.liu@...el.com>
> Reviewed-by: Tim Chen <tim.c.chen@...ux.intel.com>
> Reviewed-by: Qiuxu Zhuo <qiuxu.zhuo@...el.com>
> Reviewed-by: Yu C Chen <yu.c.chen@...el.com>
> Reviewed-by: Pan Deng <pan.deng@...el.com>
> Reviewed-by: Nanhai Zou <nanhai.zou@...el.com>
> Reviewed-by: Yuan Liu <yuan1.liu@...el.com>
> Signed-off-by: Tianyou Li <tianyou.li@...el.com>
> ---

Thanks for all your work on this and sorry for being slower with
review the last month.

While I was in the shower I was thinking about how much I hate
zone->contiguous + the pageblock walking, and how we could just get
rid of it.

You know, just what you do while having a relaxing shower.


And I was wondering:

(a) in which case would we have zone_spanned_pages == zone_present_pages
and the zone *not* being contiguous? I assume this just cannot happen,
otherwise BUG.

(b) in which case would we have zone_spanned_pages != zone_present_pages
and the zone *being* contiguous? I assume in some cases where we have small
holes within a pageblock?


Reading the doc of __pageblock_pfn_to_page(), there are some weird
scenarios with holes in pageblocks.



I.e., on my notebook I have

$ cat /proc/zoneinfo  | grep -E "Node|spanned|present"
Node 0, zone      DMA
         spanned  4095
         present  3999
Node 0, zone    DMA32
         spanned  1044480
         present  439600
Node 0, zone   Normal
         spanned  7798784
         present  7798784
Node 0, zone  Movable
         spanned  0
         present  0
Node 0, zone   Device
         spanned  0
         present  0


For the most important zone regarding compaction, ZONE_NORMAL, it would be good enough.

We certainly don't care about detecting contigous for the DMA zone. For DMA32, I would suspect
that it is not detected as contigous either way, because the holes are just way too large?


So we could maybe do (completely untested):


 From 69093e5811b532812fde52b55a42dcb24d6e09dd Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@...nel.org>
Date: Sat, 7 Feb 2026 11:45:21 +0100
Subject: [PATCH] tmp

Signed-off-by: David Hildenbrand (Arm) <david@...nel.org>
---
  include/linux/mmzone.h | 25 +++++++++++++++++++++++--
  mm/internal.h          |  8 +-------
  mm/memory_hotplug.c    | 11 +----------
  mm/mm_init.c           | 25 -------------------------
  4 files changed, 25 insertions(+), 44 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fc5d6c88d2f0..7c80df343cfd 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1051,8 +1051,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 +1122,29 @@ 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 contigous 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)
+{
+	/*
+	 * TODO: do we care about weird races? We could protect using a
+	 * seqcount or sth. like that (zone_span_seqbegin etc).
+	 *
+	 * Concurrent hotplug is not an issue. But likely the caller must
+	 * protect against concurrent hotunplug already? We should definitely
+	 * read these values through READ_ONCE and update them through
+	 * WRITE_ONCE().
+	 */
+	return zone->spanned_pages == zone->present_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..790a8839b5d8 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -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 {
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 2a809cd8e7fa..78115fb5808b 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -2263,28 +2263,6 @@ void __init init_cma_pageblock(struct page *page)
  }
  #endif
  
-void set_zone_contiguous(struct zone *zone)
-{
-	unsigned long block_start_pfn = zone->zone_start_pfn;
-	unsigned long block_end_pfn;
-
-	block_end_pfn = pageblock_end_pfn(block_start_pfn);
-	for (; block_start_pfn < zone_end_pfn(zone);
-			block_start_pfn = block_end_pfn,
-			 block_end_pfn += pageblock_nr_pages) {
-
-		block_end_pfn = min(block_end_pfn, zone_end_pfn(zone));
-
-		if (!__pageblock_pfn_to_page(block_start_pfn,
-					     block_end_pfn, zone))
-			return;
-		cond_resched();
-	}
-
-	/* We confirm that there is no hole */
-	zone->contiguous = true;
-}
-
  /*
   * Check if a PFN range intersects multiple zones on one or more
   * NUMA nodes. Specify the @nid argument if it is known that this
@@ -2347,9 +2325,6 @@ void __init page_alloc_init_late(void)
  	for_each_node_state(nid, N_MEMORY)
  		shuffle_free_memory(NODE_DATA(nid));
  
-	for_each_populated_zone(zone)
-		set_zone_contiguous(zone);
-
  	/* Initialize page ext after all struct pages are initialized. */
  	if (deferred_struct_pages)
  		page_ext_init();
-- 
2.43.0



If we would want to cover the cases with "holes in zone, but there is a struct page and it's
assigned to the zone", all we would have to do is manually track them (during boot only,
cannot happen during memory hotplug) in zone->absent pages. That value would never change.

Then we would have instead:

static inline bool zone_is_contiguous(const struct zone *zone)
{
	return zone->spanned_pages == zone->present_pages + zone->absent_pages;
}


I don't think we could just use "absent" as calculated in calculate_node_totalpages,
because I assume it could include "too many" things, not just these holes in pageblocks.


At least reading zone_absent_pages_in_node(), likely the value could return
* Pages that will not have a struct page in case of larger holes
* mirrored_kernelcore oddities

We'd need a reliably "absent pages that have a struct page that belongs to this zone".

Maybe Mike knows how to easily obtain that there to just set zone->absent_pages.

If we really need that optimization for these cases.

-- 
Cheers,

David

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ