Re: [PATCH v2 03/17] mm/mm_init: skip initializing shared vmemmap tail pages

[Muchun Song]

> On Jul 30, 2026, at 22:32, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
> 
> On Thu, Jul 30, 2026 at 09:09:00PM +0800, Muchun Song wrote:
>>> On 2026-07-26 12:30:17+08:00, Muchun Song wrote:
>>>>> 
>>>>> #else
>>>>> +struct mem_section;
>>>>> +
>>>>> #define sparse_vmemmap_init_nid_early(_nid) do {} while (0)
>>>>> #define pfn_in_present_section pfn_valid
>>>>> +static inline struct mem_section *__pfn_to_section(unsigned long pfn)
>>>>> +{
>>>>> +   return NULL;
>>>>> +}
>>>> 
>>>> I'd like to propose an alternative implementation that doesn't require
>>>> exposing the mem_section. The idea is to add a new helper function,
>>>> pfn_to_section_order(), so that for non-sparse-memory configurations,
>>>> the mem_section concept stays hidden internally. I'd really appreciate
>>>> any thoughts or concerns — if everyone is comfortable with it, I can go
>>>> ahead and implement this in the next version.
>>> 
>>> A helper that keeps mem_section hidden from !SPARSMEM makes perfect
>>> sense to me.
>>> 
>>> I'd even take it one step further and make it return how many pfns
>>> should be skipped in pfn_vmemmap_optimizable case.
>> 
>> To make sure we're on the same page, let me walk you through the specific
>> changes I have in mind. My initial plan is to introduce pfn_to_section_order,
>> and the expected diff changes are as follow to keep mem_sectionhidden from
>> !SPARSEMEM.
>> 
>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>> index dcb757b36902..0b0c2996d080 100644
>> --- a/mm/mm_init.c
>> +++ b/mm/mm_init.c
>> @@ -884,7 +884,7 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
>>                }
>> 
>>                if (pfn_vmemmap_optimizable(pfn)) {
>> -                       unsigned int order = section_order(__pfn_to_section(pfn));
>> +                       unsigned int order = pfn_to_section_order(pfn);
>> 
>>                        pfn = min(ALIGN(pfn, 1UL << order), end_pfn);
>>                        continue;
>> diff --git a/mm/sparse.h b/mm/sparse.h
>> index 030248030dc7..c5fbcdde3cee 100644
>> --- a/mm/sparse.h
>> +++ b/mm/sparse.h
>> @@ -47,6 +47,11 @@ static inline void __section_mark_present(struct mem_section *ms,
>> 
>>        ms->section_mem_map |= SECTION_MARKED_PRESENT;
>> }
>> +
>> +static inline unsigned int pfn_to_section_order(unsigned long pfn)
>> +{
>> +       return section_order(__pfn_to_section(pfn));
>> +}
>> #else
>> static inline void sparse_init(void) {}
>> #endif /* CONFIG_SPARSEMEM */
>> 
>> Since we also use __pfn_to_section in the patch 14 in this series for
> 
> I still didn't get to patch 14 :)
> 
>> !SPARSEMEM, we need to make corresponding adjustments—specifically, by using
>> pfn_to_section_order to determine whether the vmemmap of a given section is
>> optimizable. This new helper will be called from several places, so I'm afraid
>> its introduction is unavoidable.
> 
> Do you mean that section_vmemmap_optimizable() will receive pfn as a
> parameter and use pfn_to_section_order() internally?

I was originally planning to go with the this approach, but now I've come across
something that feels a bit unusual to me: a function whose name starts with section
takes a pfn as its argument instead of a mem_section.

So I started thinking about an alternative. How about we keep section_vmemmap_optimizable()
as it is, but introduce a new helper like this:

static inline bool order_vmemmap_optimizable(unsigned int order)
{
	return order >= OPTIMIZABLE_FOLIO_MIN_ORDER;
}

The main reason I'm leaning toward this design is that this new helper would be
useful in at least three places:

- Inside section_vmemmap_optimizable(), we can use it to decide whether a section
  is optimizable:

	static inline bool section_vmemmap_optimizable(const struct mem_section *section)
	{
		if (!is_power_of_2(sizeof(struct page)))
			return false;

		return order_vmemmap_optimizable(section_order(section));
	}

- In patch 14, we can call order_vmemmap_optimizable(pfn_to_section_order(pfn)) to
  make the same kind of check.

- Looking ahead, I'm hoping to unify HugeTLB's optimization checks under this
  same helper as part of my broader refactoring plan — which would also allow
  us to remove some HugeTLB-specific code. For example:

	static inline bool hugetlb_vmemmap_optimizable(const struct hstate *h)
	{
		return order_vmemmap_optimizable(huge_page_order(h));
	}

I'd really appreciate your thoughts on this — does this approach make sense to you?

Thanks
Muchun

> 
> If that's the case and pfn_to_section_order() will be used in several
> places I think it's better than to have a dedicated helper just for
> memmap_init_range().
> 
>> That said, I've also considered an alternative: introducing another helper that
>> returns the exact number of PFNs to skip, and using it solely within
>> memmap_init_range(). However, that approach doesn't seem to offer much in terms
>> of code simplification. If I'm missing something or if my reasoning doesn't align
>> with your expectations, I would really appreciate your guidance. Thank you for
>> your patience!
>> 
>> Thanks,
>> Muchun
>> 
>> 
>> 
> 
> -- 
> Sincerely yours,
> Mike.