Re: [PATCH 02/12] mm/sparse: refactor sparse_sections_init()

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Wed, Sep 09, 2026 at 03:32:55PM +0200, David Hildenbrand (Arm) wrote:
> memory_present() really identifies+prepares all early sections so the
> initialization in sparse_init() can properly iterating them to
> initialize metadata.
>
> Let's just inline memory_present() into sparse_sections_init() and
> cleaning up the code a bit while at it: make it clear that we are operating
> on pfns.
>
> Note that we call set_section_nid() now only if the section
> was not already created earlier. Now, there is no more inconsistency
> between what we (temporarily) store in ms->section_mem_map and what
> we store in our section->nid array.
>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>

In general please keep move/refactor steps separate. It makes it harder to
review when two things are going on at one time.

But I guess in this case the diff wouldn't be that different.

Anyway seems reasonable so:

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
>  mm/sparse.c | 41 +++++++++++++++++------------------------
>  1 file changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 6a6d258862904..36e3d854febc5 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -179,22 +179,27 @@ static inline unsigned long first_present_section_nr(void)
>  	return next_present_section_nr(-1);
>  }
>
> -/* Record a memory area against a node. */
> -static void __init memory_present(int nid, unsigned long start, unsigned long end)
> +void __init sparse_sections_init(void)
>  {
> -	unsigned long pfn;
> +	unsigned long pfn, start_pfn, end_pfn;
> +	int i, nid;
> +
> +	sparse_extreme_init();
>
> -	start &= PAGE_SECTION_MASK;
> -	mminit_validate_memmodel_limits(&start, &end);
> -	for (pfn = start; pfn < end; pfn += PAGES_PER_SECTION) {
> -		unsigned long section_nr = pfn_to_section_nr(pfn);
> -		struct mem_section *ms;
> +	for_each_mem_pfn_range(i, MAX_NUMNODES, &start_pfn, &end_pfn, &nid) {
> +		start_pfn &= PAGE_SECTION_MASK;
> +		mminit_validate_memmodel_limits(&start_pfn, &end_pfn);
>
> -		sparse_index_init(section_nr, nid);
> -		set_section_nid(section_nr, nid);
> +		for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> +			unsigned long section_nr = pfn_to_section_nr(pfn);
> +			struct mem_section *ms;
>
> -		ms = __nr_to_section(section_nr);
> -		if (!ms->section_mem_map) {
> +			sparse_index_init(section_nr, nid);
> +			ms = __nr_to_section(section_nr);
> +			if (ms->section_mem_map)
> +				continue;
> +
> +			set_section_nid(section_nr, nid);

So the main change seems to be calling set_section_nid() only if
!ms->section_mem_map (and obv. calculating ms earlier), as described in the
commit msg.

>  			ms->section_mem_map = sparse_encode_early_nid(nid) |
>  							SECTION_IS_ONLINE;
>  			__section_mark_present(ms, section_nr);
> @@ -202,18 +207,6 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
>  	}
>  }
>
> -/* Initialize memory section metadata for all system memory. */
> -void __init sparse_sections_init(void)
> -{
> -	unsigned long start, end;
> -	int i, nid;
> -
> -	sparse_extreme_init();
> -
> -	for_each_mem_pfn_range(i, MAX_NUMNODES, &start, &end, &nid)
> -		memory_present(nid, start, end);
> -}
> -
>  #ifndef CONFIG_SPARSEMEM_VMEMMAP
>  struct page __init *__populate_section_memmap(unsigned long pfn,
>  		unsigned long nr_pages, int nid, struct vmem_altmap *altmap,
>
> --
> 2.43.0
>

--
Cheers, Lorenzo