Re: [PATCH v2 1/6] mm/sparse-vmemmap: Fix vmemmap accounting underflow

Mike Rapoport
2026-04-15T15:53:41Z

On Wed, Apr 15, 2026 at 07:14:07PM +0800, Muchun Song wrote:
> In section_activate(), if populate_section_memmap() fails, the error
> handling path calls section_deactivate() to roll back the state. This
> causes a vmemmap accounting imbalance.
>
> Since commit c3576889d87b ("mm: fix accounting of memmap pages"),
> memmap pages are accounted for only after populate_section_memmap()
> succeeds. However, the failure path unconditionally calls
> section_deactivate(), which decreases the vmemmap count. Consequently,
> a failure in populate_section_memmap() leads to an accounting underflow,
> incorrectly reducing the system's tracked vmemmap usage.
>
> Fix this more thoroughly by moving all accounting calls into the lower
> level functions that actually perform the vmemmap allocation and freeing:
>
> - populate_section_memmap() accounts for newly allocated vmemmap pages
> - depopulate_section_memmap() unaccounts when vmemmap is freed
> - free_map_bootmem() handles early bootmem section accounting
>
> This ensures proper accounting in all code paths, including error
> handling and early section cases.
>
> Fixes: c3576889d87b ("mm: fix accounting of memmap pages")
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>

Acked-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
> mm/sparse-vmemmap.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> index 6eadb9d116e4..a7b11248b989 100644
> --- a/mm/sparse-vmemmap.c
> +++ b/mm/sparse-vmemmap.c
> @@ -656,7 +656,12 @@ static struct page * __meminit populate_section_memmap(unsigned long pfn,
> unsigned long nr_pages, int nid, struct vmem_altmap *altmap,
> struct dev_pagemap *pgmap)
> {
> - return __populate_section_memmap(pfn, nr_pages, nid, altmap, pgmap);
> + struct page *page = __populate_section_memmap(pfn, nr_pages, nid, altmap,
> + pgmap);
> +
> + memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE));
> +
> + return page;
> }
>
> static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,
> @@ -665,13 +670,17 @@ static void depopulate_section_memmap(unsigned long pfn, unsigned long nr_pages,
> unsigned long start = (unsigned long) pfn_to_page(pfn);
> unsigned long end = start + nr_pages * sizeof(struct page);
>
> + memmap_pages_add(-1L * (DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE)));
> vmemmap_free(start, end, altmap);
> }
> +
> static void free_map_bootmem(struct page *memmap)
> {
> unsigned long start = (unsigned long)memmap;
> unsigned long end = (unsigned long)(memmap + PAGES_PER_SECTION);
>
> + memmap_boot_pages_add(-1L * (DIV_ROUND_UP(PAGES_PER_SECTION * sizeof(struct page),
> + PAGE_SIZE)));
> vmemmap_free(start, end, NULL);
> }
>
> @@ -774,14 +783,10 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> * The memmap of early sections is always fully populated. See
> * section_activate() and pfn_valid() .
> */
> - if (!section_is_early) {
> - memmap_pages_add(-1L * (DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE)));
> + if (!section_is_early)
> depopulate_section_memmap(pfn, nr_pages, altmap);
> - } else if (memmap) {
> - memmap_boot_pages_add(-1L * (DIV_ROUND_UP(nr_pages * sizeof(struct page),
> - PAGE_SIZE)));
> + else if (memmap)
> free_map_bootmem(memmap);
> - }
>
> if (empty)
> ms->section_mem_map = (unsigned long)NULL;
> @@ -826,7 +831,6 @@ static struct page * __meminit section_activate(int nid, unsigned long pfn,
> section_deactivate(pfn, nr_pages, altmap);
> return ERR_PTR(-ENOMEM);
> }
> - memmap_pages_add(DIV_ROUND_UP(nr_pages * sizeof(struct page), PAGE_SIZE));
>
> return memmap;
> }
> --
> 2.20.1
>

--
Sincerely yours,
Mike.