* [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
@ 2026-03-31 11:37 Muchun Song
2026-03-31 18:34 ` Donet Tom
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Muchun Song @ 2026-03-31 11:37 UTC (permalink / raw)
To: Andrew Morton, David Hildenbrand
Cc: Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Frank van der Linden, linux-mm,
linux-kernel, muchun.song, Muchun Song
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;
}
}
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-03-31 11:37 [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path Muchun Song
@ 2026-03-31 18:34 ` Donet Tom
2026-04-01 2:28 ` Muchun Song
2026-03-31 20:10 ` Andrew Morton
2026-03-31 20:42 ` David Hildenbrand (Arm)
2 siblings, 1 reply; 13+ messages in thread
From: Donet Tom @ 2026-03-31 18:34 UTC (permalink / raw)
To: Muchun Song, Andrew Morton, David Hildenbrand
Cc: Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Frank van der Linden, linux-mm,
linux-kernel, muchun.song
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
> }
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-03-31 11:37 [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path Muchun Song
2026-03-31 18:34 ` Donet Tom
@ 2026-03-31 20:10 ` Andrew Morton
2026-03-31 21:06 ` David Hildenbrand (Arm)
2026-04-01 2:37 ` Muchun Song
2026-03-31 20:42 ` David Hildenbrand (Arm)
2 siblings, 2 replies; 13+ messages in thread
From: Andrew Morton @ 2026-03-31 20:10 UTC (permalink / raw)
To: Muchun Song
Cc: David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Frank van der Linden, linux-mm, linux-kernel, muchun.song
On Tue, 31 Mar 2026 19:37:24 +0800 Muchun Song <songmuchun@bytedance.com> 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.
Here I go again ;) Are there userspace impacts?
AI review thinks it found a different bug:
https://sashiko.dev/#/patchset/20260331113724.2080833-1-songmuchun@bytedance.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-03-31 11:37 [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path Muchun Song
2026-03-31 18:34 ` Donet Tom
2026-03-31 20:10 ` Andrew Morton
@ 2026-03-31 20:42 ` David Hildenbrand (Arm)
2026-04-01 2:41 ` Muchun Song
2 siblings, 1 reply; 13+ messages in thread
From: David Hildenbrand (Arm) @ 2026-03-31 20:42 UTC (permalink / raw)
To: Muchun Song, Andrew Morton
Cc: Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Frank van der Linden, linux-mm,
linux-kernel, muchun.song
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@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;
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
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.
--
Cheers,
David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-03-31 20:10 ` Andrew Morton
@ 2026-03-31 21:06 ` David Hildenbrand (Arm)
2026-04-01 2:37 ` Muchun Song
1 sibling, 0 replies; 13+ messages in thread
From: David Hildenbrand (Arm) @ 2026-03-31 21:06 UTC (permalink / raw)
To: Andrew Morton, Muchun Song
Cc: Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Frank van der Linden, linux-mm,
linux-kernel, muchun.song
On 3/31/26 22:10, Andrew Morton wrote:
> On Tue, 31 Mar 2026 19:37:24 +0800 Muchun Song <songmuchun@bytedance.com> 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.
>
> Here I go again ;) Are there userspace impacts?
>
> AI review thinks it found a different bug:
> https://sashiko.dev/#/patchset/20260331113724.2080833-1-songmuchun@bytedance.com
It speculates about a BUG_ON that we should easily hit with hugetlb
vmemmap optimization during boot I think (which we don't)? That
naturally make me skeptical.
But sounds like something to double check independently of this patch here.
--
Cheers,
David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-03-31 18:34 ` Donet Tom
@ 2026-04-01 2:28 ` Muchun Song
0 siblings, 0 replies; 13+ messages in thread
From: Muchun Song @ 2026-04-01 2:28 UTC (permalink / raw)
To: Donet Tom
Cc: Muchun Song, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Frank van der Linden, linux-mm,
linux-kernel
> 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.
>
> - Donet
>
>> }
>> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-03-31 20:10 ` Andrew Morton
2026-03-31 21:06 ` David Hildenbrand (Arm)
@ 2026-04-01 2:37 ` Muchun Song
1 sibling, 0 replies; 13+ messages in thread
From: Muchun Song @ 2026-04-01 2:37 UTC (permalink / raw)
To: Andrew Morton
Cc: Muchun Song, David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Frank van der Linden, linux-mm, linux-kernel
> On Apr 1, 2026, at 04:10, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 31 Mar 2026 19:37:24 +0800 Muchun Song <songmuchun@bytedance.com> 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.
>
> Here I go again ;) Are there userspace impacts?
Those pre-inited sections (HugeTLB pages) are not activated. However, such
failures are extremely rare, so I don't see any major issues.
>
> AI review thinks it found a different bug:
> https://sashiko.dev/#/patchset/20260331113724.2080833-1-songmuchun@bytedance.com
I don't think the issue reported by AI is a real problem, because the
allocation of sparse_usagebuf has already taken these hugetlb sections
into account.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-03-31 20:42 ` David Hildenbrand (Arm)
@ 2026-04-01 2:41 ` Muchun Song
2026-04-01 7:25 ` David Hildenbrand (Arm)
0 siblings, 1 reply; 13+ messages in thread
From: Muchun Song @ 2026-04-01 2:41 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Muchun Song, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Frank van der Linden, linux-mm, linux-kernel
> On Apr 1, 2026, at 04:42, David Hildenbrand (Arm) <david@kernel.org> 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@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;
>
>
> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
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@bytedance.com>
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-04-01 2:41 ` Muchun Song
@ 2026-04-01 7:25 ` David Hildenbrand (Arm)
2026-04-01 7:28 ` Muchun Song
2026-04-02 7:37 ` Donet Tom
0 siblings, 2 replies; 13+ messages in thread
From: David Hildenbrand (Arm) @ 2026-04-01 7:25 UTC (permalink / raw)
To: Muchun Song
Cc: Muchun Song, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Frank van der Linden, linux-mm, linux-kernel
On 4/1/26 04:41, Muchun Song wrote:
>
>
>> On Apr 1, 2026, at 04:42, David Hildenbrand (Arm) <david@kernel.org> 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@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;
>>
>>
>> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
>
> 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.
Don't use BUG_ON, use actual panic(). :)
And yes, we should rip out that handling.
--
Cheers,
David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-04-01 7:25 ` David Hildenbrand (Arm)
@ 2026-04-01 7:28 ` Muchun Song
2026-04-02 7:37 ` Donet Tom
1 sibling, 0 replies; 13+ messages in thread
From: Muchun Song @ 2026-04-01 7:28 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Muchun Song, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Frank van der Linden, linux-mm, linux-kernel
> On Apr 1, 2026, at 15:25, David Hildenbrand (Arm) <david@kernel.org> wrote:
>
> On 4/1/26 04:41, Muchun Song wrote:
>>
>>
>>> On Apr 1, 2026, at 04:42, David Hildenbrand (Arm) <david@kernel.org> 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@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;
>>>
>>>
>>> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
>>
>> 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.
>
> Don't use BUG_ON, use actual panic(). :)
Got it.
>
> And yes, we should rip out that handling.
Agree.
>
> --
> Cheers,
>
> David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-04-01 7:25 ` David Hildenbrand (Arm)
2026-04-01 7:28 ` Muchun Song
@ 2026-04-02 7:37 ` Donet Tom
2026-04-02 7:56 ` David Hildenbrand (Arm)
1 sibling, 1 reply; 13+ messages in thread
From: Donet Tom @ 2026-04-02 7:37 UTC (permalink / raw)
To: David Hildenbrand (Arm), Muchun Song
Cc: Muchun Song, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Frank van der Linden, linux-mm, linux-kernel
On 4/1/26 12:55 PM, David Hildenbrand (Arm) wrote:
> On 4/1/26 04:41, Muchun Song wrote:
>>
>>> On Apr 1, 2026, at 04:42, David Hildenbrand (Arm) <david@kernel.org> 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@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;
>>>
>>> Acked-by: David Hildenbrand (Arm) <david@kernel.org>
>> 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.
> Don't use BUG_ON, use actual panic(). :)
Apologies if I’m missing something, but would it make sense to check the
section validity in memmap_init() instead of panicking? Is there a
reason this is avoided?
-Donet
>
> And yes, we should rip out that handling.
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-04-02 7:37 ` Donet Tom
@ 2026-04-02 7:56 ` David Hildenbrand (Arm)
2026-04-02 12:12 ` Donet Tom
0 siblings, 1 reply; 13+ messages in thread
From: David Hildenbrand (Arm) @ 2026-04-02 7:56 UTC (permalink / raw)
To: Donet Tom, Muchun Song
Cc: Muchun Song, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Frank van der Linden, linux-mm, linux-kernel
On 4/2/26 09:37, Donet Tom wrote:
>
> On 4/1/26 12:55 PM, David Hildenbrand (Arm) wrote:
>> On 4/1/26 04:41, Muchun Song wrote:
>>>
>>> Thanks.
>>>
>>> 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.
>> Don't use BUG_ON, use actual panic(). :)
>
>
> Apologies if I’m missing something, but would it make sense to check the
> section validity in memmap_init() instead of panicking? Is there a
> reason this is avoided?
Not sure I understand your question, but trying to handle memmap
allocations during early boot should be removed. If that ever happens
something is fundamentally flawed.
--
Cheers,
David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path
2026-04-02 7:56 ` David Hildenbrand (Arm)
@ 2026-04-02 12:12 ` Donet Tom
0 siblings, 0 replies; 13+ messages in thread
From: Donet Tom @ 2026-04-02 12:12 UTC (permalink / raw)
To: David Hildenbrand (Arm), Muchun Song
Cc: Muchun Song, Andrew Morton, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Frank van der Linden, linux-mm, linux-kernel
On 4/2/26 1:26 PM, David Hildenbrand (Arm) wrote:
> On 4/2/26 09:37, Donet Tom wrote:
>> On 4/1/26 12:55 PM, David Hildenbrand (Arm) wrote:
>>> On 4/1/26 04:41, Muchun Song wrote:
>>>> Thanks.
>>>>
>>>> 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.
>>> Don't use BUG_ON, use actual panic(). :)
>>
>> Apologies if I’m missing something, but would it make sense to check the
>> section validity in memmap_init() instead of panicking? Is there a
>> reason this is avoided?
> Not sure I understand your question, but trying to handle memmap
> allocations during early boot should be removed. If that ever happens
> something is fundamentally flawed.
Thank you for the clarification—I understand now.
-Donet
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-04-02 12:12 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-31 11:37 [PATCH] mm/sparse: fix preinited section_mem_map clobbering on failure path Muchun Song
2026-03-31 18:34 ` Donet Tom
2026-04-01 2:28 ` Muchun Song
2026-03-31 20:10 ` Andrew Morton
2026-03-31 21:06 ` David Hildenbrand (Arm)
2026-04-01 2:37 ` Muchun Song
2026-03-31 20:42 ` David Hildenbrand (Arm)
2026-04-01 2:41 ` Muchun Song
2026-04-01 7:25 ` David Hildenbrand (Arm)
2026-04-01 7:28 ` Muchun Song
2026-04-02 7:37 ` Donet Tom
2026-04-02 7:56 ` David Hildenbrand (Arm)
2026-04-02 12:12 ` Donet Tom
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox