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

[Muchun Song]

> On Apr 1, 2026, at 04:42, David Hildenbrand (Arm) <david@xxxxxxxxxx> wrote:
> 
> On 3/31/26 13:37, 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@xxxxxxxxxxxxx>
>> ---
>> 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;
> 
> 
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

Thanks.

> 
> I have some cleanup patches lying around that cleanup that code heavily.
> I think I get rid of this questionable "failed to allocate" case entirely.

It's truly a coincidence — I also have a piece of code locally
that does something similar. Since allocation failure would also
affect subsequent startup processes, I simply made it panic when
allocation fails.

    mm: panic on memory allocation failure in sparse_init_nid()

    When vmemmap pages allocation or usemap allocation fails, sparse_init_nid()
    currently only marks the corresponding section as non-present. However,
    subsequent code like memmap_init() iterating over PFNs does not check for
    non-present sections, leading to invalid memory access (additional,
    subsection_map_init() accessing the unallocated usemap as well).

    It is complex to audit and fix all boot-time PFN iterators to handle these
    partially initialized sections correctly. Since vmemmap and usemap allocation
    failures are extremely rare during early boot, the more appropriate approach
    is to expose the problem as early as possible.

    Therefore, use BUG_ON() to panic immediately if allocation fails, instead of
    attempting a partial recovery that leads to obscure crashes later.

    Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>

diff --git a/mm/sparse.c b/mm/sparse.c
index 1b017026925e..bb0b86a4f3ef 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -354,19 +354,14 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,
                                   unsigned long map_count)
 {
        unsigned long pnum;
-       struct page *map;
-       struct mem_section *ms;
-
-       if (sparse_usage_init(nid, map_count)) {
-               pr_err("%s: node[%d] usemap allocation failed", __func__, nid);
-               goto failed;
-       }

+       BUG_ON(sparse_usage_init(nid, map_count));
        sparse_buffer_init(map_count * section_map_size(), nid);

        sparse_vmemmap_init_nid_early(nid);

        for_each_present_section_nr(pnum_begin, pnum) {
+               struct mem_section *ms;
                unsigned long pfn = section_nr_to_pfn(pnum);

                if (pnum >= pnum_end)
@@ -374,16 +369,11 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,

                ms = __nr_to_section(pnum);
                if (!preinited_vmemmap_section(ms)) {
+                       struct page *map;
+
                        map = __populate_section_memmap(pfn, PAGES_PER_SECTION,
-                                       nid, NULL, NULL);
-                       if (!map) {
-                               pr_err("%s: node[%d] memory map backing failed. Some memory will not be available.",
-                                      __func__, nid);
-                               pnum_begin = pnum;
-                               sparse_usage_fini();
-                               sparse_buffer_fini();
-                               goto failed;
-                       }
+                                                       nid, NULL, NULL);
+                       BUG_ON(!map);
                        memmap_boot_pages_add(DIV_ROUND_UP(PAGES_PER_SECTION * sizeof(struct page),
                                                           PAGE_SIZE));
                        sparse_init_early_section(nid, map, pnum, 0);
@@ -391,19 +381,6 @@ static void __init sparse_init_nid(int nid, unsigned long pnum_begin,
        }
        sparse_usage_fini();
        sparse_buffer_fini();
-       return;
-failed:
-       /*
-        * We failed to allocate, mark all the following pnums as not present,
-        * except the ones already initialized earlier.
-        */
-       for_each_present_section_nr(pnum_begin, pnum) {
-               if (pnum >= pnum_end)
-                       break;
-               ms = __nr_to_section(pnum);
-               if (!preinited_vmemmap_section(ms))
-                       ms->section_mem_map = 0;
-       }
 }

 /*

> 
> -- 
> Cheers,
> 
> David