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

[Lance Yang]

On 2026/10/1 10:05, Muchun Song wrote:
> > On Oct 1, 2026, at 00:04, Lance Yang <lance.yang@xxxxxxxxx> 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@xxxxxxxxx/
> > Suggested-by: Muchun Song <muchun.song@xxxxxxxxx>
> > Signed-off-by: Lance Yang <lance.yang@xxxxxxxxx>
> > ---
> > 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.

Thanks, Muchun!

> > 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@xxxxxxxxx>

Cheers! Lance