Re: [PATCH v5 09/17] mm/sparse: initialize memory sections earlier

[David Hildenbrand (Arm)]

On 8/25/26 10:46, Muchun Song wrote:
> Upcoming HugeTLB bootmem changes need sparsemem section metadata before
> the HugeTLB bootmem allocation path runs. The memory sections are
> initialized from sparse_init(), which is called too late for that setup.
> 
> Move the code that initializes sparsemem section metadata for memblock
> ranges into mm_core_init_early(), before free_area_init() and the HugeTLB
> bootmem setup. Rename the helper to sparse_sections_init() so the new
> caller describes the sparsemem-specific initialization step.
> 
> This is a preparatory change.
> 
> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> Reviewed-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> ---
> v5:
> - Collect Reviewed-by from Mike Rapoport
> 
> v2:
> - Rename the helper to sparse_sections_init() to describe the section
>   metadata initialization (suggested by Mike Rapoport)
> - Fix the !SPARSEMEM stub name so SPARSEMEM=n builds compile
>   (reported by Sashiko)
> ---
>  mm/mm_init.c |  1 +
>  mm/sparse.c  | 10 ++--------
>  mm/sparse.h  |  2 ++
>  3 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index 0317fb781e77..e2a16d833635 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -2642,6 +2642,7 @@ void __init mm_core_init_early(void)
>  {
>  	kho_memory_init_early();
>  
> +	sparse_sections_init();
>  	free_area_init();
>  
>  	hugetlb_cma_reserve();
> diff --git a/mm/sparse.c b/mm/sparse.c
> index e6cb67ca9c8d..439802e6a6ad 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -191,12 +191,8 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
>  	}
>  }
>  
> -/*
> - * Mark all memblocks as present using memory_present().
> - * This is a convenience function that is useful to mark all of the systems
> - * memory as present during initialization.
> - */
> -static void __init memblocks_present(void)
> +/* Initialize memory section metadata for all system memory. */

I'd drop this comment, it's not entirely accurate. A lot more initialization of
the sections happens later.

What this function mostly does is

(a) Allocate sparse-exteme metadata
(b) Temporarily store the nid and mark the section for being present and online

I have patches to get rid of __section_mark_present() and just set them all to
SECTION_IS_EARLY|SECTION_IS_ONLINE such that we can just iterate early sections
and get rid of that PRESENT monstrosity.

Apart from that LGTM, even though I don't particularly enjoy splitting us now
having two init entry points

sparse_init

and

sparse_sections_init


Where it's rather unclear from the name what is actually happening.

-- 
Cheers,

David