Re: [PATCH v4] powerpc/hugetlb: Disable gigantic hugepages if fadump is active

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> The fadump kernel boots with limited memory solely to collect the kernel
> core dump. Having gigantic hugepages in the fadump kernel is of no use.

Sure got it.

> Many times, the fadump kernel encounters OOM (Out of Memory) issues if
> gigantic hugepages are allocated.
>
> To address this, disable gigantic hugepages if fadump is active by
> returning early from arch_hugetlb_valid_size() using
> hugepages_supported(). When fadump is active, the global variable
> hugetlb_disabled is set to true, which is later used by the
> PowerPC-specific hugepages_supported() function to determine hugepage
> support.
>
> Returning early from arch_hugetlb_vali_size() not only disables
> gigantic hugepages but also avoids unnecessary hstate initialization for
> every hugepage size supported by the platform.
>
> kernel logs related to hugepages with this patch included:
> kernel argument passed: hugepagesz=1G hugepages=1
>
> First kernel: gigantic hugepage got allocated
> ==============================================
>
> dmesg | grep -i "hugetlb"
> -------------------------
> HugeTLB: registered 1.00 GiB page size, pre-allocated 1 pages
> HugeTLB: 0 KiB vmemmap can be freed for a 1.00 GiB page
> HugeTLB: registered 2.00 MiB page size, pre-allocated 0 pages
> HugeTLB: 0 KiB vmemmap can be freed for a 2.00 MiB page
>
> $ cat /proc/meminfo | grep -i "hugetlb"
> -------------------------------------
> Hugetlb:         1048576 kB

Was this tested with patch [1] in your local tree?

[1]: https://web.git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=d629d7a8efc33

IIUC, this patch [1] disables the boot time allocation of hugepages.
Isn't it also disabling the boot time allocation for gigantic huge pages
passed by the cmdline params like hugepagesz=1G and hugepages=2 ?


> HugeTLB: registered 1.00 GiB page size, pre-allocated 1 pages
This print comes from report_hugepages(). The only place from where
report_hugepages() gets called is hugetlb_init(). hugetlb_init() is what
is responsible for hugepages & gigantic hugepage allocations of the
passed kernel cmdline params.

But hugetlb_init() already checks for hugepages_supported() in the very
beginning. So I am not sure whether we need this extra patch to disable
gigantic hugepages allocation by the kernel cmdline params like
hugepagesz=1G and hugepages=2 type of options.

Hence I was wondering if you had this patch [1] in your tree when you were
testing this?

But I may be missing something. Could you please help clarify on whether
we really need this patch to disable gigantic hugetlb page allocations?

>
> Fadump kernel: gigantic hugepage not allocated
> ===============================================
>
> dmesg | grep -i "hugetlb"
> -------------------------
> [    0.000000] HugeTLB: unsupported hugepagesz=1G
> [    0.000000] HugeTLB: hugepages=1 does not follow a valid hugepagesz, ignoring
> [    0.706375] HugeTLB support is disabled!
> [    0.773530] hugetlbfs: disabling because there are no supported hugepage sizes
>
> $ cat /proc/meminfo | grep -i "hugetlb"
> ----------------------------------
> <Nothing>
>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Ritesh Harjani (IBM)" <ritesh.list@gmail.com>

I guess the extra " in the above was not adding me in the cc list.
Hence I missed to see this patch early.

-ritesh


> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
> Changelog:
>
> v1:
> https://lore.kernel.org/all/20250121150419.1342794-1-sourabhjain@linux.ibm.com/
>
> v2:
> https://lore.kernel.org/all/20250124103220.111303-1-sourabhjain@linux.ibm.com/
>  - disable gigantic hugepage in arch code, arch_hugetlb_valid_size()
>
> v3:
> https://lore.kernel.org/all/20250125104928.88881-1-sourabhjain@linux.ibm.com/
>  - Do not modify the initialization of the shift variable
>
> v4:
> - Update commit message to include how hugepages_supported() detects
>   hugepages support when fadump is active
> - Add Reviewed-by tag
> - NO functional change
>
> ---
>  arch/powerpc/mm/hugetlbpage.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 6b043180220a..88cfd182db4e 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -138,6 +138,9 @@ bool __init arch_hugetlb_valid_size(unsigned long size)
>  	int shift = __ffs(size);
>  	int mmu_psize;
>
> +	if (!hugepages_supported())
> +		return false;
> +
>  	/* Check that it is a page size supported by the hardware and
>  	 * that it fits within pagetable and slice limits. */
>  	if (size <= PAGE_SIZE || !is_power_of_2(size))
> --
> 2.48.1