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

[Sourabh Jain]

Hello Ritesh,

Thanks for the review.


On 02/03/25 12:05, Ritesh Harjani (IBM) wrote:
> Sourabh Jain <sourabhjain at 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 ?

Yes, I had the patch [1] in my tree.

My understanding is that gigantic pages are allocated before normal huge 
pages.

In mm/hugetlb.c:hugepages_setup(), we have:

if (hugetlb_max_hstate && hstate_is_gigantic(parsed_hstate))
     hugetlb_hstate_alloc_pages(parsed_hstate);

I believe the above code allocates memory for gigantic pages, and 
mm/hugetlb.c:hugetlb_init()
is called later because it is a subsys_initcall.

So, by the time the kernel reaches hugetlb_init(), the gigantic pages 
are already
allocated. Isn't that right?

With this understanding, this patch avoids populating hstate so that 
gigantic
huge page allocation fails for the fadump kernel.

Please let me know your opinion.


>
>> 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 at linux.ibm.com>
>> Cc: Madhavan Srinivasan <maddy at linux.ibm.com>
>> Cc: Mahesh Salgaonkar <mahesh at linux.ibm.com>
>> Cc: Michael Ellerman <mpe at ellerman.id.au>
>> Cc: Ritesh Harjani (IBM)" <ritesh.list at 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.

Thanks for pointing it. I will fix it.

>
> -ritesh
>
>
>> Reviewed-by: Christophe Leroy <christophe.leroy at csgroup.eu>
>> Signed-off-by: Sourabh Jain <sourabhjain at 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