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

[Sourabh Jain]

On 27/01/25 13:18, Christophe Leroy wrote:
>
>
> Le 25/01/2025 à 10:32, Sourabh Jain a écrit :
>> Hello Christophe
>>
>>
>> On 24/01/25 19:44, Christophe Leroy wrote:
>>>
>>>
>>> Le 24/01/2025 à 11:32, Sourabh Jain a écrit :
>>>> 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.
>>>> 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(). hugepages_supported() returns false if 
>>>> fadump is
>>>> active.
>>>>
>>>> Returning early from arch_hugetlb_valid_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
>>>>
>>>> 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>
>>>> Signed-off-by: Sourabh Jain <sourabhjain at linux.ibm.com>
>>>> ---
>>>>
>>>> Changelog:
>>>>
>>>> v1:
>>>> https://eur01.safelinks.protection.outlook.com/? 
>>>> url=https%3A%2F%2Flore.kernel.org%2Fall%2F20250121150419.1342794-1- 
>>>> sourabhjain%40linux.ibm.com%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C747d8db131e44ea472cf08dd3d234a0f%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638733943957107236%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=wArHG9ozTqBJWRbp850rkU7ioemZjai3TEf7nZGm4h0%3D&reserved=0 
>>>>
>>>>
>>>> v2:
>>>>   - disable gigantic hugepage in arch code, arch_hugetlb_valid_size()
>>>>
>>>> ---
>>>>   arch/powerpc/mm/hugetlbpage.c | 8 ++++++--
>>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/ 
>>>> hugetlbpage.c
>>>> index 6b043180220a..087a8df32416 100644
>>>> --- a/arch/powerpc/mm/hugetlbpage.c
>>>> +++ b/arch/powerpc/mm/hugetlbpage.c
>>>> @@ -135,8 +135,12 @@ int __init alloc_bootmem_huge_page(struct 
>>>> hstate *h, int nid)
>>>>     bool __init arch_hugetlb_valid_size(unsigned long size)
>>>>   {
>>>> -    int shift = __ffs(size);
>>>> -    int mmu_psize;
>>>> +    int shift, mmu_psize;
>>>> +
>>>> +    if (!hugepages_supported())
>>>> +        return false;
>>>> +
>>>> +    shift = __ffs(size);
>>>
>>> Why change the declaration/init of shift ?
>>
>> I did this to avoid running code that wasn't necessary.
>
> The compiler does it for you, when you look at the assembly you see 
> the calculation is done only when needed.

Yeah I didn't consider that. Thanks.

- Sourabh Jain