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

[Christophe Leroy]

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.


> 
>>
>> It should be enough to leave things as they are and just add
>>
>>     if (!hugepages_supported())
>>         return false;
> 
> Sure.
> 
>>
>>
>>>         /* Check that it is a page size supported by the hardware and
>>>        * that it fits within pagetable and slice limits. */
>>
> 
> Thanks for the review.
> 
> - Sourabh Jain