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

["David Hildenbrand (Arm)" <david@kernel.org>]

On 9/9/26 15:33, 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().
> 
> Should we use READ_ONCE/WRITE_ONCE with __highest_used_section_nr?
> Probably, something for another day.
> 
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
>  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;
>  }

Sashiko says that section_nr could be used uninitialized if the loop body
doesn't run.

Certainly a very high regression.

In a universe where a machine can boot without any memory?

"This could occur if mminit_validate_memmodel_limits() clamps
start_pfn and end_pfn to the same value, or if for_each_mem_pfn_range()
yields no memory ranges."

I think this can be ignored.


Sashiko also thinks it found a pre-existing issue when updating
ms->section_mem_map in sparse_init_one_section() when subsections already exist.

That would be the case during ZONE_DEVICE hotplug only. Likely we should skip
the sparse_init_one_section() if already initialized. I'll add this to my todo,
but it has low priority.

-- 
Cheers,

David