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

Donet Tom
2026-04-02T06:43:00Z

[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]

On 4/1/26 7:58 AM, Muchun Song wrote:
>
>> On Apr 1, 2026, at 02:34, Donet Tom<donettom@linux.ibm.com> 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<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?
> 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 <donettom@linux.ibm.com>

>
> Thanks.
>
>> - Donet
>>
>>> }
>>> }
>

[-- Attachment #2: Type: text/html, Size: 4688 bytes --]