Re: [PATCH 1/1] mm/memory_hotplug: fix missing rollback in __add_pages()

[Muchun Song <muchun.song@linux.dev>]

> On Oct 1, 2026, at 00:04, Lance Yang <lance.yang@linux.dev> wrote:
> 
> __add_pages() returns on a sparse_add_section() failure without removing
> the sections already added in the same request.
> 
> For memremap_pages(), the failed range is not counted in pgmap->nr_range,
> so memunmap_pages() skips it. The sections already added in that range
> retain their vmemmap mappings and subsection bits. Retrying a
> section-aligned range can then fail with -EEXIST.
> 
> Save the initial PFN and remove [start_pfn, pfn) on failure. For a vmemmap
> population failure, section_activate() already cleans up the current
> section, so the rollback excludes it. If the first section fails,
> __remove_pages() receives an empty range and does nothing.
> 
> Link: https://lore.kernel.org/all/BAD58999-1EDD-4A37-ABA0-DB1BD8AB3453@linux.dev/
> Suggested-by: Muchun Song <muchun.song@linux.dev>
> Signed-off-by: Lance Yang <lance.yang@linux.dev>
> ---
> No Fixes tag, as I couldn't identify the commit that introduced this issue.

Hi Lance,

LLMs are quite good at tracing this kind of code history, so I used one
to go through the relevant commits and identify the correct Fixes tag.

Fixes: ba72b4c8cf60 ("mm/sparsemem: support sub-section hotplug")

Before that commit, __add_pages() ignored -EEXIST and continued with
the remaining sections. After a partial failure, a retry could therefore
reuse the vmemmap of sections added by the failed attempt and continue
with the later sections.

Commit ba72b4c8cf60 made -EEXIST a hard error because
sparse_add_section() began using it to report an actual subsection
collision. That semantic change was correct, but without rolling back
the sections added earlier in the request, stale subsection bits make the
retry stop at the first previously added section.

The vmemmap removal infrastructure had already been added by commit
0197518cd367 ("memory-hotplug: remove memmap of sparse-vmemmap").
Therefore, ba72b4c8cf60 appears to be the commit that made this bug
observable in the way described by this patch.

> 
> mm/memory_hotplug.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 796af1028ee2..ca4656698148 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -380,6 +380,7 @@ EXPORT_SYMBOL_GPL(pfn_to_online_page);
> int __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> struct mhp_params *params)
> {
> + 	const unsigned long start_pfn = pfn;
> 	const unsigned long end_pfn = pfn + nr_pages;
> 	unsigned long cur_nr_pages;
> int err;
> @@ -413,8 +414,11 @@ int __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
>   SECTION_ALIGN_UP(pfn + 1) - pfn);
> 		err = sparse_add_section(nid, pfn, cur_nr_pages, altmap,
> 					params->pgmap);
> - 		if (err)
> + 		if (err) {
> + 			__remove_pages(start_pfn, pfn - start_pfn, altmap,
> +       				params->pgmap);
> 			break;
> + 		}
> 		cond_resched();
> 	}
> 	vmemmap_populate_print_last();

Acked-by: Muchun Song <muchun.song@linux.dev>

Thanks,
Muchun

> -- 
> 2.39.3
>