* Re: [PATCH 04/17] mm/mm_init: skip initializing shared vmemmap tail pages
[not found] ` <20260702093821.2740183-5-songmuchun@bytedance.com>
@ 2026-07-09 10:45 ` Mike Rapoport
2026-07-09 12:31 ` Muchun Song
0 siblings, 1 reply; 6+ messages in thread
From: Mike Rapoport @ 2026-07-09 10:45 UTC (permalink / raw)
To: Muchun Song
Cc: Andrew Morton, Oscar Salvador, David Hildenbrand, linux-mm,
Vlastimil Babka, Lorenzo Stoakes, Michal Hocko, linux-kernel,
muchun.song
Hi Muchun,
Below are some preliminary comments, I'm planning to spend more time on
review next week.
On Thu, Jul 02, 2026 at 05:38:08PM +0800, Muchun Song wrote:
> memmap_init_range() initializes every struct page in the target range.
> For compound pages with vmemmap optimization, the tail struct pages are
> backed by a shared vmemmap page.
>
> Initializing those tail struct pages would overwrite the shared
> vmemmap page contents, so users such as HugeTLB have to open-code
> follow-up handling to restore the metadata afterwards.
>
> Use the section's compound page order to detect struct pages that fall
> into the shared tail vmemmap range and skip their initialization in
> memmap_init_range(). Still initialize the pageblock migratetypes for
> the skipped range so the surrounding setup remains intact.
>
> This is a preparatory change for consolidating handling across users of
> vmemmap optimization, and it also avoids redundant initialization of
> shared tail vmemmap pages during early boot.
>
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
> include/linux/mmzone.h | 4 ++++
> mm/internal.h | 16 ++++++++++++++++
> mm/mm_init.c | 25 +++++++++++++++++++------
> 3 files changed, 39 insertions(+), 6 deletions(-)
>
> @@ -673,19 +673,21 @@ static inline void fixup_hashdist(void)
> static inline void fixup_hashdist(void) {}
> #endif /* CONFIG_NUMA */
>
> -#if defined(CONFIG_ZONE_DEVICE) || defined(CONFIG_DEFERRED_STRUCT_PAGE_INIT)
> static __meminit void pageblock_migratetype_init_range(unsigned long pfn,
> - unsigned long nr_pages, int migratetype, bool atomic)
> + unsigned long nr_pages, int migratetype, bool isolate, bool atomic)
What is isolate parameter for?
> {
> const unsigned long end = pfn + nr_pages;
>
> for (pfn = pageblock_align(pfn); pfn < end; pfn += pageblock_nr_pages) {
> - init_pageblock_migratetype(pfn_to_page(pfn), migratetype, false);
> + init_pageblock_migratetype(pfn_to_page(pfn), migratetype, isolate);
> +#ifdef CONFIG_SPARSEMEM
> if (!atomic && IS_ALIGNED(pfn, PAGES_PER_SECTION))
> +#else
> + if (!atomic && IS_ALIGNED(pfn, MAX_FOLIO_NR_PAGES))
> +#endif
Let's trigger cond_resched() on some defined number of iterations or some
memory size chunk, e.g PAGES_PER_128M or even PAGES_PER_1G.
> cond_resched();
> }
> }
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 08/17] mm/sparse: mark memory sections present earlier
[not found] ` <20260702093821.2740183-9-songmuchun@bytedance.com>
@ 2026-07-09 10:54 ` Mike Rapoport
2026-07-09 12:35 ` Muchun Song
0 siblings, 1 reply; 6+ messages in thread
From: Mike Rapoport @ 2026-07-09 10:54 UTC (permalink / raw)
To: Muchun Song
Cc: Andrew Morton, Oscar Salvador, David Hildenbrand, linux-mm,
Vlastimil Babka, Lorenzo Stoakes, Michal Hocko, linux-kernel,
muchun.song
On Thu, Jul 02, 2026 at 05:38:12PM +0800, Muchun Song wrote:
> Upcoming HugeTLB bootmem changes need sparsemem section metadata before
> the HugeTLB bootmem allocation path runs. The memblock ranges are marked
> present from sparse_init(), which is called too late for that setup.
It's not only that memblock regions are marked present, but it actually
initializes mem_section's for the present memory ...
> Move the code that marks memblock ranges present into
> mm_core_init_early(), before free_area_init() and the HugeTLB bootmem
> setup. Rename the helper to sparse_memblock_present() to make the new
... so let's name this function to reflect that.
How about sparse_sections_init()?
> caller describe the sparsemem-specific initialization step.
>
> This is a preparatory change.
>
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
> mm/internal.h | 2 ++
> mm/mm_init.c | 1 +
> mm/sparse.c | 4 +---
> 3 files changed, 4 insertions(+), 3 deletions(-)
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 04/17] mm/mm_init: skip initializing shared vmemmap tail pages
2026-07-09 10:45 ` [PATCH 04/17] mm/mm_init: skip initializing shared vmemmap tail pages Mike Rapoport
@ 2026-07-09 12:31 ` Muchun Song
2026-07-09 13:05 ` Mike Rapoport
0 siblings, 1 reply; 6+ messages in thread
From: Muchun Song @ 2026-07-09 12:31 UTC (permalink / raw)
To: Mike Rapoport
Cc: Muchun Song, Andrew Morton, Oscar Salvador, David Hildenbrand,
linux-mm, Vlastimil Babka, Lorenzo Stoakes, Michal Hocko,
linux-kernel
> On Jul 9, 2026, at 18:45, Mike Rapoport <rppt@kernel.org> wrote:
>
> Hi Muchun,
Hi,
>
> Below are some preliminary comments, I'm planning to spend more time on
> review next week.
Thanks for the early feedback! Looking forward to more review next week.
>
> On Thu, Jul 02, 2026 at 05:38:08PM +0800, Muchun Song wrote:
>> memmap_init_range() initializes every struct page in the target range.
>> For compound pages with vmemmap optimization, the tail struct pages are
>> backed by a shared vmemmap page.
>>
>> Initializing those tail struct pages would overwrite the shared
>> vmemmap page contents, so users such as HugeTLB have to open-code
>> follow-up handling to restore the metadata afterwards.
>>
>> Use the section's compound page order to detect struct pages that fall
>> into the shared tail vmemmap range and skip their initialization in
>> memmap_init_range(). Still initialize the pageblock migratetypes for
>> the skipped range so the surrounding setup remains intact.
>>
>> This is a preparatory change for consolidating handling across users of
>> vmemmap optimization, and it also avoids redundant initialization of
>> shared tail vmemmap pages during early boot.
>>
>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>> ---
>> include/linux/mmzone.h | 4 ++++
>> mm/internal.h | 16 ++++++++++++++++
>> mm/mm_init.c | 25 +++++++++++++++++++------
>> 3 files changed, 39 insertions(+), 6 deletions(-)
>>
>> @@ -673,19 +673,21 @@ static inline void fixup_hashdist(void)
>> static inline void fixup_hashdist(void) {}
>> #endif /* CONFIG_NUMA */
>>
>> -#if defined(CONFIG_ZONE_DEVICE) || defined(CONFIG_DEFERRED_STRUCT_PAGE_INIT)
>> static __meminit void pageblock_migratetype_init_range(unsigned long pfn,
>> - unsigned long nr_pages, int migratetype, bool atomic)
>> + unsigned long nr_pages, int migratetype, bool isolate, bool atomic)
>
> What is isolate parameter for?
I've re-examined the code, and you're right that the isolate parameter is technically
redundant for our current use case, as memmap_init_zone_range() passes false.
The rationale behind keeping it is future-proofing. The ultimate goal of a generic
HVO is to support arbitrary huge pages, not just HugeTLB. I decoupled this as a
parameter to prevent potential regressions down the road; if a developer leverages
this for other huge page types in the future, they won't inadvertently break
things by forgetting to update a hardcoded false in init_pageblock_migratetype(),
especially since memmap_init_range() natively accepts an isolate parameter.
Of course, we could also just delete this parameter for now and add it back later if
needed. I think both approaches work.
Which way are you leaning?
>
>> {
>> const unsigned long end = pfn + nr_pages;
>>
>> for (pfn = pageblock_align(pfn); pfn < end; pfn += pageblock_nr_pages) {
>> - init_pageblock_migratetype(pfn_to_page(pfn), migratetype, false);
>> + init_pageblock_migratetype(pfn_to_page(pfn), migratetype, isolate);
>> +#ifdef CONFIG_SPARSEMEM
>> if (!atomic && IS_ALIGNED(pfn, PAGES_PER_SECTION))
>> +#else
>> + if (!atomic && IS_ALIGNED(pfn, MAX_FOLIO_NR_PAGES))
>> +#endif
>
> Let's trigger cond_resched() on some defined number of iterations or some
> memory size chunk, e.g PAGES_PER_128M or even PAGES_PER_1G.
Yes, that's for the best. I was really struggling to choose a suitable macro
for this earlier, but I realized it's a difficult thing to get right. I'm leaning
toward selecting PAGES_PER_1G instead.
Muchun,
Thanks.
>
>> cond_resched();
>> }
>> }
>
> --
> Sincerely yours,
> Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 08/17] mm/sparse: mark memory sections present earlier
2026-07-09 10:54 ` [PATCH 08/17] mm/sparse: mark memory sections present earlier Mike Rapoport
@ 2026-07-09 12:35 ` Muchun Song
0 siblings, 0 replies; 6+ messages in thread
From: Muchun Song @ 2026-07-09 12:35 UTC (permalink / raw)
To: Mike Rapoport
Cc: Muchun Song, Andrew Morton, Oscar Salvador, David Hildenbrand,
linux-mm, Vlastimil Babka, Lorenzo Stoakes, Michal Hocko,
linux-kernel
> On Jul 9, 2026, at 18:54, Mike Rapoport <rppt@kernel.org> wrote:
>
> On Thu, Jul 02, 2026 at 05:38:12PM +0800, Muchun Song wrote:
>> Upcoming HugeTLB bootmem changes need sparsemem section metadata before
>> the HugeTLB bootmem allocation path runs. The memblock ranges are marked
>> present from sparse_init(), which is called too late for that setup.
>
> It's not only that memblock regions are marked present, but it actually
> initializes mem_section's for the present memory ...
Right.
>
>> Move the code that marks memblock ranges present into
>> mm_core_init_early(), before free_area_init() and the HugeTLB bootmem
>> setup. Rename the helper to sparse_memblock_present() to make the new
>
> ... so let's name this function to reflect that.
>
> How about sparse_sections_init()?
Make sense. This name really captures the function's purpose well.
I'll go ahead and use it.
Muchun,
Thanks.
>
>> caller describe the sparsemem-specific initialization step.
>>
>> This is a preparatory change.
>>
>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>> ---
>> mm/internal.h | 2 ++
>> mm/mm_init.c | 1 +
>> mm/sparse.c | 4 +---
>> 3 files changed, 4 insertions(+), 3 deletions(-)
>
> --
> Sincerely yours,
> Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 04/17] mm/mm_init: skip initializing shared vmemmap tail pages
2026-07-09 12:31 ` Muchun Song
@ 2026-07-09 13:05 ` Mike Rapoport
2026-07-09 13:23 ` Muchun Song
0 siblings, 1 reply; 6+ messages in thread
From: Mike Rapoport @ 2026-07-09 13:05 UTC (permalink / raw)
To: Muchun Song
Cc: Muchun Song, Andrew Morton, Oscar Salvador, David Hildenbrand,
linux-mm, Vlastimil Babka, Lorenzo Stoakes, Michal Hocko,
linux-kernel
On Thu, Jul 09, 2026 at 08:31:31PM +0800, Muchun Song wrote:
> > On Jul 9, 2026, at 18:45, Mike Rapoport <rppt@kernel.org> wrote:
> > On Thu, Jul 02, 2026 at 05:38:08PM +0800, Muchun Song wrote:
> >> memmap_init_range() initializes every struct page in the target range.
> >> For compound pages with vmemmap optimization, the tail struct pages are
> >> backed by a shared vmemmap page.
> >>
> >> Initializing those tail struct pages would overwrite the shared
> >> vmemmap page contents, so users such as HugeTLB have to open-code
> >> follow-up handling to restore the metadata afterwards.
> >>
> >> Use the section's compound page order to detect struct pages that fall
> >> into the shared tail vmemmap range and skip their initialization in
> >> memmap_init_range(). Still initialize the pageblock migratetypes for
> >> the skipped range so the surrounding setup remains intact.
> >>
> >> This is a preparatory change for consolidating handling across users of
> >> vmemmap optimization, and it also avoids redundant initialization of
> >> shared tail vmemmap pages during early boot.
> >>
> >> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> >> ---
> >> include/linux/mmzone.h | 4 ++++
> >> mm/internal.h | 16 ++++++++++++++++
> >> mm/mm_init.c | 25 +++++++++++++++++++------
> >> 3 files changed, 39 insertions(+), 6 deletions(-)
> >>
> >> @@ -673,19 +673,21 @@ static inline void fixup_hashdist(void)
> >> static inline void fixup_hashdist(void) {}
> >> #endif /* CONFIG_NUMA */
> >>
> >> -#if defined(CONFIG_ZONE_DEVICE) || defined(CONFIG_DEFERRED_STRUCT_PAGE_INIT)
> >> static __meminit void pageblock_migratetype_init_range(unsigned long pfn,
> >> - unsigned long nr_pages, int migratetype, bool atomic)
> >> + unsigned long nr_pages, int migratetype, bool isolate, bool atomic)
> >
> > What is isolate parameter for?
>
> I've re-examined the code, and you're right that the isolate parameter is technically
> redundant for our current use case, as memmap_init_zone_range() passes false.
>
> The rationale behind keeping it is future-proofing. The ultimate goal of a generic
> HVO is to support arbitrary huge pages, not just HugeTLB. I decoupled this as a
> parameter to prevent potential regressions down the road; if a developer leverages
> this for other huge page types in the future, they won't inadvertently break
> things by forgetting to update a hardcoded false in init_pageblock_migratetype(),
> especially since memmap_init_range() natively accepts an isolate parameter.
>
> Of course, we could also just delete this parameter for now and add it back later if
> needed. I think both approaches work.
>
> Which way are you leaning?
I'd drop it for now and would revisit when there would be a new HVO user.
Even one boolean means it's hard to tell from a call site what is the
intention of the flag, two make it completely confusing :)
> >> {
> >> const unsigned long end = pfn + nr_pages;
> >>
> >> for (pfn = pageblock_align(pfn); pfn < end; pfn += pageblock_nr_pages) {
> >> - init_pageblock_migratetype(pfn_to_page(pfn), migratetype, false);
> >> + init_pageblock_migratetype(pfn_to_page(pfn), migratetype, isolate);
> >> +#ifdef CONFIG_SPARSEMEM
> >> if (!atomic && IS_ALIGNED(pfn, PAGES_PER_SECTION))
> >> +#else
> >> + if (!atomic && IS_ALIGNED(pfn, MAX_FOLIO_NR_PAGES))
> >> +#endif
> >
> > Let's trigger cond_resched() on some defined number of iterations or some
> > memory size chunk, e.g PAGES_PER_128M or even PAGES_PER_1G.
>
> Yes, that's for the best. I was really struggling to choose a suitable macro
> for this earlier, but I realized it's a difficult thing to get right. I'm leaning
> toward selecting PAGES_PER_1G instead.
Yeah, PAGES_PER_1G makes sense to me too.
> Muchun,
> Thanks.
>
> >
> >> cond_resched();
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 04/17] mm/mm_init: skip initializing shared vmemmap tail pages
2026-07-09 13:05 ` Mike Rapoport
@ 2026-07-09 13:23 ` Muchun Song
0 siblings, 0 replies; 6+ messages in thread
From: Muchun Song @ 2026-07-09 13:23 UTC (permalink / raw)
To: Mike Rapoport
Cc: Muchun Song, Andrew Morton, Oscar Salvador, David Hildenbrand,
linux-mm, Vlastimil Babka, Lorenzo Stoakes, Michal Hocko,
linux-kernel
> On Jul 9, 2026, at 21:05, Mike Rapoport <rppt@kernel.org> wrote:
>
> On Thu, Jul 09, 2026 at 08:31:31PM +0800, Muchun Song wrote:
>>> On Jul 9, 2026, at 18:45, Mike Rapoport <rppt@kernel.org> wrote:
>>> On Thu, Jul 02, 2026 at 05:38:08PM +0800, Muchun Song wrote:
>>>> memmap_init_range() initializes every struct page in the target range.
>>>> For compound pages with vmemmap optimization, the tail struct pages are
>>>> backed by a shared vmemmap page.
>>>>
>>>> Initializing those tail struct pages would overwrite the shared
>>>> vmemmap page contents, so users such as HugeTLB have to open-code
>>>> follow-up handling to restore the metadata afterwards.
>>>>
>>>> Use the section's compound page order to detect struct pages that fall
>>>> into the shared tail vmemmap range and skip their initialization in
>>>> memmap_init_range(). Still initialize the pageblock migratetypes for
>>>> the skipped range so the surrounding setup remains intact.
>>>>
>>>> This is a preparatory change for consolidating handling across users of
>>>> vmemmap optimization, and it also avoids redundant initialization of
>>>> shared tail vmemmap pages during early boot.
>>>>
>>>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>>>> ---
>>>> include/linux/mmzone.h | 4 ++++
>>>> mm/internal.h | 16 ++++++++++++++++
>>>> mm/mm_init.c | 25 +++++++++++++++++++------
>>>> 3 files changed, 39 insertions(+), 6 deletions(-)
>>>>
>>>> @@ -673,19 +673,21 @@ static inline void fixup_hashdist(void)
>>>> static inline void fixup_hashdist(void) {}
>>>> #endif /* CONFIG_NUMA */
>>>>
>>>> -#if defined(CONFIG_ZONE_DEVICE) || defined(CONFIG_DEFERRED_STRUCT_PAGE_INIT)
>>>> static __meminit void pageblock_migratetype_init_range(unsigned long pfn,
>>>> - unsigned long nr_pages, int migratetype, bool atomic)
>>>> + unsigned long nr_pages, int migratetype, bool isolate, bool atomic)
>>>
>>> What is isolate parameter for?
>>
>> I've re-examined the code, and you're right that the isolate parameter is technically
>> redundant for our current use case, as memmap_init_zone_range() passes false.
>>
>> The rationale behind keeping it is future-proofing. The ultimate goal of a generic
>> HVO is to support arbitrary huge pages, not just HugeTLB. I decoupled this as a
>> parameter to prevent potential regressions down the road; if a developer leverages
>> this for other huge page types in the future, they won't inadvertently break
>> things by forgetting to update a hardcoded false in init_pageblock_migratetype(),
>> especially since memmap_init_range() natively accepts an isolate parameter.
>>
>> Of course, we could also just delete this parameter for now and add it back later if
>> needed. I think both approaches work.
>>
>> Which way are you leaning?
>
> I'd drop it for now and would revisit when there would be a new HVO user.
> Even one boolean means it's hard to tell from a call site what is the
> intention of the flag, two make it completely confusing :)
No problem. I'll drop it next version.
Muchun,
Thanks.
>
>>>> {
>>>> const unsigned long end = pfn + nr_pages;
>>>>
>>>> for (pfn = pageblock_align(pfn); pfn < end; pfn += pageblock_nr_pages) {
>>>> - init_pageblock_migratetype(pfn_to_page(pfn), migratetype, false);
>>>> + init_pageblock_migratetype(pfn_to_page(pfn), migratetype, isolate);
>>>> +#ifdef CONFIG_SPARSEMEM
>>>> if (!atomic && IS_ALIGNED(pfn, PAGES_PER_SECTION))
>>>> +#else
>>>> + if (!atomic && IS_ALIGNED(pfn, MAX_FOLIO_NR_PAGES))
>>>> +#endif
>>>
>>> Let's trigger cond_resched() on some defined number of iterations or some
>>> memory size chunk, e.g PAGES_PER_128M or even PAGES_PER_1G.
>>
>> Yes, that's for the best. I was really struggling to choose a suitable macro
>> for this earlier, but I realized it's a difficult thing to get right. I'm leaning
>> toward selecting PAGES_PER_1G instead.
>
> Yeah, PAGES_PER_1G makes sense to me too.
>
>> Muchun,
>> Thanks.
>>
>>>
>>>> cond_resched();
>
> --
> Sincerely yours,
> Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-09 13:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260702093821.2740183-1-songmuchun@bytedance.com>
[not found] ` <20260702093821.2740183-5-songmuchun@bytedance.com>
2026-07-09 10:45 ` [PATCH 04/17] mm/mm_init: skip initializing shared vmemmap tail pages Mike Rapoport
2026-07-09 12:31 ` Muchun Song
2026-07-09 13:05 ` Mike Rapoport
2026-07-09 13:23 ` Muchun Song
[not found] ` <20260702093821.2740183-9-songmuchun@bytedance.com>
2026-07-09 10:54 ` [PATCH 08/17] mm/sparse: mark memory sections present earlier Mike Rapoport
2026-07-09 12:35 ` Muchun Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox