Re: [PATCH 08/12] mm/sparse: move __highest_used_section_nr handling

[Lorenzo Stoakes (ARM)]

On Wed, Sep 09, 2026 at 03:33:01PM +0200, David Hildenbrand (Arm) wrote:
> Let's move it to sparse_init_one_section(). However, to keep early
> boot processing working, we also have to initialize it in
> sparse_sections_init().

A why might be nice :)

I guess preparing for removal of __section_mark_present()?

Also good to have arguments as to why this is equivalent of previous
behaviour.

E.g. higher pfn = higher section nr so naturally the highest is the one you
end up wtih at the end of sparse_sections_init()?

>
> Should we use READ_ONCE/WRITE_ONCE with __highest_used_section_nr?
> Probably, something for another day.

It might be worth expanding this a bit. Do multiple threads read/write this
concurrently?

No functional change intended here or is one intended? :)

Before it was:

mm_core_init_early() -> sparse_sections_init() -> __section_mark_present()
                          sparse_add_section() -> __section_mark_present()

Now:

                                                 mm_core_init_early() -> sparse_sections_init() [early]
                                                 sparse_add_section() -> sparse_init_one_section()

But also called from mm_core_init_early():

sparse_init() -> sparse_metadata_init() -> sparse_metadata_init_nid() -> sparse_init_one_section()

Are both required?

Might be worth digging into why a bit here.


>
> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> ---
>  mm/sparse.c | 5 +++--
>  mm/sparse.h | 6 +++---
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 2b41ae36f20b8..2d0f2db34f4cf 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -177,7 +177,7 @@ static inline unsigned long first_present_section_nr(void)
>
>  void __init sparse_sections_init(void)
>  {
> -	unsigned long pfn, start_pfn, end_pfn;
> +	unsigned long pfn, start_pfn, end_pfn, section_nr;
>  	int i, nid;
>
>  	sparse_extreme_init();
> @@ -187,9 +187,9 @@ void __init sparse_sections_init(void)
>  		mminit_validate_memmodel_limits(&start_pfn, &end_pfn);
>
>  		for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> -			unsigned long section_nr = pfn_to_section_nr(pfn);
>  			struct mem_section *ms;
>
> +			section_nr = pfn_to_section_nr(pfn);
>  			sparse_index_init(section_nr, nid);
>  			ms = __nr_to_section(section_nr);
>  			if (ms->section_mem_map)
> @@ -201,6 +201,7 @@ void __init sparse_sections_init(void)
>  			__section_mark_present(ms, section_nr);
>  		}
>  	}
> +	__highest_used_section_nr = section_nr;
>  }
>
>  #ifndef CONFIG_SPARSEMEM_VMEMMAP
> diff --git a/mm/sparse.h b/mm/sparse.h
> index 7c5d82ceb7142..a3af4967fd5c5 100644
> --- a/mm/sparse.h
> +++ b/mm/sparse.h
> @@ -97,6 +97,9 @@ static inline void sparse_init_one_section(struct mem_section *ms,
>
>  	BUILD_BUG_ON(SECTION_MAP_LAST_BIT > PFN_SECTION_SHIFT);
>
> +	if (section_nr > __highest_used_section_nr)
> +		__highest_used_section_nr = section_nr;
> +

Could also be:

	section_nr = max(section_nr, __highest_used_section_nr);

But not a big deal.


>  	/*
>  	 * We encode the start PFN of the section into the mem_map such that
>  	 * page_to_pfn() on !CONFIG_SPARSEMEM_VMEMMAP can simply subtract it
> @@ -114,9 +117,6 @@ static inline void sparse_init_one_section(struct mem_section *ms,
>  static inline void __section_mark_present(struct mem_section *ms,
>  		unsigned long section_nr)
>  {
> -	if (section_nr > __highest_used_section_nr)
> -		__highest_used_section_nr = section_nr;
> -
>  	ms->section_mem_map |= SECTION_MARKED_PRESENT;
>  }
>
>
> --
> 2.43.0
>

--
Cheers, Lorenzo