[PATCH] mm/memory_hotplug: factor out node_is_memoryless()

[PATCH] mm/memory_hotplug: factor out node_is_memoryless()

[Gregory Price]

A memoryless node neither spans present pages (populated or ZONE_DEVICE)
nor has an offline-but-added memory block still linked to it in sysfs.

try_offline_node() presently open-codes this memoryless check.

Pull that into a node_is_memoryless() helper and pull the existing
check_no_memblock_for_node_cb() helper ahead of the add/online path
so it's clearer what is happening here.

No functional change.

Signed-off-by: Gregory Price <gourry@gourry.net>
---
 mm/memory_hotplug.c | 60 +++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 29 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 226ab9cb078ad..d0e94057682af 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1491,6 +1491,36 @@ static int create_altmaps_and_memory_blocks(int nid, struct memory_group *group,
 	return ret;
 }
 
+static int check_no_memblock_for_node_cb(struct memory_block *mem, void *arg)
+{
+	int nid = *(int *)arg;
+
+	/*
+	 * If a memory block belongs to multiple nodes, the stored nid is not
+	 * reliable. However, such blocks are always online (e.g., cannot get
+	 * offlined) and, therefore, are still spanned by the node.
+	 */
+	return mem->nid == nid ? -EEXIST : 0;
+}
+
+/* Caller must hold the memory hotplug lock for this check. */
+static bool node_is_memoryless(int nid)
+{
+	/*
+	 * A node still spanning pages (especially ZONE_DEVICE) is not
+	 * memoryless.  A node spans memory after move_pfn_range_to_zone(),
+	 * e.g. once a memory block has been onlined.
+	 */
+	if (node_spanned_pages(nid))
+		return false;
+	/*
+	 * Offline memory blocks may not be spanned by the node yet, but they
+	 * link to it in sysfs and can be onlined later, so the node is not
+	 * memoryless while any remain.
+	 */
+	return !for_each_memory_block(&nid, check_no_memblock_for_node_cb);
+}
+
 /*
  * NOTE: The caller must call lock_device_hotplug() to serialize hotplug
  * and online/offline operations (triggered e.g. by sysfs).
@@ -2214,18 +2244,6 @@ static int check_cpu_on_node(int nid)
 	return 0;
 }
 
-static int check_no_memblock_for_node_cb(struct memory_block *mem, void *arg)
-{
-	int nid = *(int *)arg;
-
-	/*
-	 * If a memory block belongs to multiple nodes, the stored nid is not
-	 * reliable. However, such blocks are always online (e.g., cannot get
-	 * offlined) and, therefore, are still spanned by the node.
-	 */
-	return mem->nid == nid ? -EEXIST : 0;
-}
-
 /**
  * try_offline_node
  * @nid: the node ID
@@ -2237,23 +2255,7 @@ static int check_no_memblock_for_node_cb(struct memory_block *mem, void *arg)
  */
 void try_offline_node(int nid)
 {
-	int rc;
-
-	/*
-	 * If the node still spans pages (especially ZONE_DEVICE), don't
-	 * offline it. A node spans memory after move_pfn_range_to_zone(),
-	 * e.g., after the memory block was onlined.
-	 */
-	if (node_spanned_pages(nid))
-		return;
-
-	/*
-	 * Especially offline memory blocks might not be spanned by the
-	 * node. They will get spanned by the node once they get onlined.
-	 * However, they link to the node in sysfs and can get onlined later.
-	 */
-	rc = for_each_memory_block(&nid, check_no_memblock_for_node_cb);
-	if (rc)
+	if (!node_is_memoryless(nid))
 		return;
 
 	if (check_cpu_on_node(nid))
-- 
2.53.0-Meta

Re: [PATCH] mm/memory_hotplug: factor out node_is_memoryless()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Unused function warnings when CONFIG_MEMORY_HOTREMOVE is disabled.

Pre-existing issues:
- [High] Data race on `node_spanned_pages` between ACPI CPU hot-remove and ZONE_DEVICE memory hotplug due to missing memory hotplug lock.
--

commit 239875f97a26527846083c3d6ab82ee6b800ce0a
Author: Gregory Price <gourry@gourry.net>

mm/memory_hotplug: factor out node_is_memoryless()

A memoryless node neither spans present pages (populated or ZONE_DEVICE)
nor has an offline-but-added memory block still linked to it in sysfs.

try_offline_node() presently open-codes this memoryless check.

Pull that into a node_is_memoryless() helper and pull the existing
check_no_memblock_for_node_cb() helper ahead of the add/online path
so it's clearer what is happening here.

> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 226ab9cb078ad..d0e94057682af 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1491,6 +1491,36 @@ static int create_altmaps_and_memory_blocks(int nid, struct memory_group *group,
>  	return ret;
>  }
>  
> +static int check_no_memblock_for_node_cb(struct memory_block *mem, void *arg)
> +{
> +	int nid = *(int *)arg;
> +
> +	/*
> +	 * If a memory block belongs to multiple nodes, the stored nid is not
> +	 * reliable. However, such blocks are always online (e.g., cannot get
> +	 * offlined) and, therefore, are still spanned by the node.
> +	 */
> +	return mem->nid == nid ? -EEXIST : 0;
> +}
> +
> +/* Caller must hold the memory hotplug lock for this check. */
> +static bool node_is_memoryless(int nid)
> +{
> +	/*
> +	 * A node still spanning pages (especially ZONE_DEVICE) is not
> +	 * memoryless.  A node spans memory after move_pfn_range_to_zone(),
> +	 * e.g. once a memory block has been onlined.
> +	 */
> +	if (node_spanned_pages(nid))

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but can
this lockless read of node_spanned_pages() race with ZONE_DEVICE memory
hotplug?

The comment explicitly requires the memory hotplug lock, but the ACPI CPU
hot-remove path in drivers/acpi/acpi_processor.c:acpi_processor_post_eject()
calls try_offline_node() while holding only device_hotplug_lock():

drivers/acpi/acpi_processor.c:acpi_processor_post_eject() {
    ...
    cpus_write_unlock();
    cpu_maps_update_done();

    try_offline_node(cpu_to_node(pr->id));
}

Since ZONE_DEVICE memory hotplug (via memremap_pages()) modifies
node_spanned_pages under mem_hotplug_lock() without holding
device_hotplug_lock(), could this race result in a NUMA node with active
ZONE_DEVICE memory being erroneously offlined and unregistered from sysfs?

> +		return false;
> +	/*
> +	 * Offline memory blocks may not be spanned by the node yet, but they
> +	 * link to it in sysfs and can be onlined later, so the node is not
> +	 * memoryless while any remain.
> +	 */
> +	return !for_each_memory_block(&nid, check_no_memblock_for_node_cb);
> +}

[Severity: Low]
Does moving check_no_memblock_for_node_cb() and node_is_memoryless() outside
of the #ifdef CONFIG_MEMORY_HOTREMOVE block cause -Wunused-function warnings
when CONFIG_MEMORY_HOTREMOVE is disabled?

Their only caller, try_offline_node(), is conditionally compiled inside that
block, so these helpers would be unused in builds without it.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902195507.88655-1-gourry@gourry.net?part=1

Re: [PATCH] mm/memory_hotplug: factor out node_is_memoryless()

[Gregory Price]

On Wed, Sep 02, 2026 at 03:55:07PM -0400, Gregory Price wrote:
> A memoryless node neither spans present pages (populated or ZONE_DEVICE)
> nor has an offline-but-added memory block still linked to it in sysfs.
> 
> try_offline_node() presently open-codes this memoryless check.
> 
> Pull that into a node_is_memoryless() helper and pull the existing
> check_no_memblock_for_node_cb() helper ahead of the add/online path
> so it's clearer what is happening here.
> 
> No functional change.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---
>  mm/memory_hotplug.c | 60 +++++++++++++++++++++++----------------------
>  1 file changed, 31 insertions(+), 29 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 226ab9cb078ad..d0e94057682af 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1491,6 +1491,36 @@ static int create_altmaps_and_memory_blocks(int nid, struct memory_group *group,
>  	return ret;
>  }
>  
> +static int check_no_memblock_for_node_cb(struct memory_block *mem, void *arg)
> +{
> +	int nid = *(int *)arg;
> +
> +	/*
> +	 * If a memory block belongs to multiple nodes, the stored nid is not
> +	 * reliable. However, such blocks are always online (e.g., cannot get
> +	 * offlined) and, therefore, are still spanned by the node.
> +	 */
> +	return mem->nid == nid ? -EEXIST : 0;
> +}
> +
> +/* Caller must hold the memory hotplug lock for this check. */
> +static bool node_is_memoryless(int nid)
> +{
> +	/*
> +	 * A node still spanning pages (especially ZONE_DEVICE) is not
> +	 * memoryless.  A node spans memory after move_pfn_range_to_zone(),
> +	 * e.g. once a memory block has been onlined.
> +	 */
> +	if (node_spanned_pages(nid))
> +		return false;

browsing sashiko feedback:

[Severity: High]
This is a pre-existing issue and was not introduced by this patch, but can
this lockless read of node_spanned_pages() race with ZONE_DEVICE memory
hotplug?

---

This seems legit and worth addressing (other notes are addressible as
well, but this is maybe noteworthy).

I actually have some old patches sandbagged that tried to marry the
ZONE_DEVICE hotplug pattern through mm/memory_hotplug.c rather than its
separare entry-point.  Might be worth a revisit.

I don't know that I want to predicate this particular fix on this patch
but it's worth a think.

~Gregory