Re: [RFC v3 3/3] fadump: Move fadump_cma_init to setup_arch() after initmem_init()

[Madhavan Srinivasan]

On 10/11/24 8:30 PM, Ritesh Harjani (IBM) wrote:
> During early init CMA_MIN_ALIGNMENT_BYTES can be PAGE_SIZE,
> since pageblock_order is still zero and it gets initialized
> later during initmem_init() e.g.
> setup_arch() -> initmem_init() -> sparse_init() -> set_pageblock_order()
> 
> One such use case where this causes issues is -
> early_setup() -> early_init_devtree() -> fadump_reserve_mem() -> fadump_cma_init()
> 
> This causes CMA memory alignment check to be bypassed in
> cma_init_reserved_mem(). Then later cma_activate_area() can hit
> a VM_BUG_ON_PAGE(pfn & ((1 << order) - 1)) if the reserved memory
> area was not pageblock_order aligned.
> 
> Fix it by moving the fadump_cma_init() after initmem_init(),
> where other such cma reservations also gets called.
> 
> <stack trace>
> ==============
> page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x10010
> flags: 0x13ffff800000000(node=1|zone=0|lastcpupid=0x7ffff) CMA
> raw: 013ffff800000000 5deadbeef0000100 5deadbeef0000122 0000000000000000
> raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
> page dumped because: VM_BUG_ON_PAGE(pfn & ((1 << order) - 1))
> ------------[ cut here ]------------
> kernel BUG at mm/page_alloc.c:778!
> 
> Call Trace:
> __free_one_page+0x57c/0x7b0 (unreliable)
> free_pcppages_bulk+0x1a8/0x2c8
> free_unref_page_commit+0x3d4/0x4e4
> free_unref_page+0x458/0x6d0
> init_cma_reserved_pageblock+0x114/0x198
> cma_init_reserved_areas+0x270/0x3e0
> do_one_initcall+0x80/0x2f8
> kernel_init_freeable+0x33c/0x530
> kernel_init+0x34/0x26c
> ret_from_kernel_user_thread+0x14/0x1c
> 

Changes looks fine to me.
Reviewed-by: Madhavan Srinivasan <maddy@xxxxxxxxxxxxx>

> Fixes: 11ac3e87ce09 ("mm: cma: use pageblock_order as the single alignment")
> Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
> Reported-by: Sachin P Bappalige <sachinpb@xxxxxxxxxxxxx>
> Acked-by: Hari Bathini <hbathini@xxxxxxxxxxxxx>
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx>
> ---
>  arch/powerpc/include/asm/fadump.h  | 7 +++++++
>  arch/powerpc/kernel/fadump.c       | 6 +-----
>  arch/powerpc/kernel/setup-common.c | 6 ++++--
>  3 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
> index ef40c9b6972a..3638f04447f5 100644
> --- a/arch/powerpc/include/asm/fadump.h
> +++ b/arch/powerpc/include/asm/fadump.h
> @@ -34,4 +34,11 @@ extern int early_init_dt_scan_fw_dump(unsigned long node, const char *uname,
>  				      int depth, void *data);
>  extern int fadump_reserve_mem(void);
>  #endif
> +
> +#if defined(CONFIG_FA_DUMP) && defined(CONFIG_CMA)
> +void fadump_cma_init(void);
> +#else
> +static inline void fadump_cma_init(void) { }
> +#endif
> +
>  #endif /* _ASM_POWERPC_FADUMP_H */
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index ffaec625b7a8..c42f89862893 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -78,7 +78,7 @@ static struct cma *fadump_cma;
>   * But for some reason even if it fails we still have the memory reservation
>   * with us and we can still continue doing fadump.
>   */
> -static void __init fadump_cma_init(void)
> +void __init fadump_cma_init(void)
>  {
>  	unsigned long long base, size, end;
>  	int rc;
> @@ -139,8 +139,6 @@ static void __init fadump_cma_init(void)
>  		fw_dump.boot_memory_size >> 20);
>  	return;
>  }
> -#else
> -static void __init fadump_cma_init(void) { }
>  #endif /* CONFIG_CMA */
>  
>  /*
> @@ -642,8 +640,6 @@ int __init fadump_reserve_mem(void)
>  
>  		pr_info("Reserved %lldMB of memory at %#016llx (System RAM: %lldMB)\n",
>  			(size >> 20), base, (memblock_phys_mem_size() >> 20));
> -
> -		fadump_cma_init();
>  	}
>  
>  	return ret;
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 4bd2f87616ba..9f1e6f2e299e 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -996,9 +996,11 @@ void __init setup_arch(char **cmdline_p)
>  	initmem_init();
>  
>  	/*
> -	 * Reserve large chunks of memory for use by CMA for KVM and hugetlb. These must
> -	 * be called after initmem_init(), so that pageblock_order is initialised.
> +	 * Reserve large chunks of memory for use by CMA for fadump, KVM and
> +	 * hugetlb. These must be called after initmem_init(), so that
> +	 * pageblock_order is initialised.
>  	 */
> +	fadump_cma_init();
>  	kvm_cma_reserve();
>  	gigantic_hugetlb_cma_reserve();
>