Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path

Donet Tom
2026-03-31T18:34:59Z

Hi Muchun

On 3/31/26 5:07 PM, Muchun Song wrote:
> sparse_init_nid() is careful to leave alone every section whose vmemmap
> has already been set up by sparse_vmemmap_init_nid_early(); it only
> clears section_mem_map for the rest:
>
> if (!preinited_vmemmap_section(ms))
> ms->section_mem_map = 0;
>
> A leftover line after that conditional block
>
> ms->section_mem_map = 0;
>
> was supposed to be deleted but was missed in the failure path, causing the
> field to be overwritten for all sections when memory allocation fails,
> effectively destroying the pre-initialization check.
>
> Drop the stray assignment so that preinited sections retain their
> already valid state.
>
> Fixes: d65917c42373 ("mm/sparse: allow for alternate vmemmap section init at boot")
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
> mm/sparse.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index c2eb36bfb86d..3a14b733bf71 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -584,7 +584,6 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,
> ms = __nr_to_section(pnum);
> if (!preinited_vmemmap_section(ms))
> ms->section_mem_map = 0;
> - ms->section_mem_map = 0;

This looks correct to me.

I have a couple of questions:

1. As I understand, section_mem_map initially stores the nid
during early boot, and later it stores a pointer to an
array of struct page. In sparse_init_nid(), the struct page
array is stored in section_mem_map via
sparse_init_early_section(). If
__populate_section_memmap() fails, we are  clearing the nid
stored in section_mem_map right?

2. Another question: if sparse_init_nid() fails for some
sections, there is no retry mechanism to add them again,
correct?

3. when ms->section_mem_map is set to 0
for a pre-initialized section, does it only affect the
pre-initialization check, or could it lead to other issues?

- Donet

> }
> }
>