On 4/1/26 7:58 AM, Muchun Song wrote: > >> On Apr 1, 2026, at 02:34, Donet Tom wrote: >> >> Hi Muchun > Hi, > >> 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 >>> --- >>> 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? > Right. > >> 2. Another question: if sparse_init_nid() fails for some >> sections, there is no retry mechanism to add them again, >> correct? > Right. > >> 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? >> > Only affect pre-initialization. No other issues. Thanks for the clarification. This looks good to me. Reviewed by: Donet Tom > > Thanks. > >> - Donet >> >>> } >>> } >