[PATCH] powerpc/book3s64/hugetlb: Fix disabling hugetlb when fadump is active

[Sourabh Jain]

Hello Ritesh,


On 17/12/24 02:19, Ritesh Harjani (IBM) wrote:
> Ritesh Harjani (IBM) <ritesh.list at gmail.com> writes:
>
>> Sourabh Jain <sourabhjain at linux.ibm.com> writes:
>>
>>> Commit 8597538712eb ("powerpc/fadump: Do not use hugepages when fadump
>>> is active") disabled hugetlb support when fadump is active by returning
>>> early from hugetlbpage_init():arch/powerpc/mm/hugetlbpage.c and not
>>> populating hpage_shift/HPAGE_SHIFT.
>>>
>>> Later, commit 2354ad252b66 ("powerpc/mm: Update default hugetlb size
>>> early") moved the allocation of hpage_shift/HPAGE_SHIFT to early boot,
>>> which inadvertently re-enabled hugetlb support when fadump is active.
>>>
>>> Fix this by implementing hugepages_supported() on powerpc. This ensures
>>> that disabling hugetlb for the fadump kernel is independent of
>>> hpage_shift/HPAGE_SHIFT.
>>>
>> Thanks for describing the history of the changes clearly.
>>
>>> Fixes: 2354ad252b66 ("powerpc/mm: Update default hugetlb size early")
>>> CC: Aneesh Kumar K.V <aneesh.kumar at kernel.org>
>>> 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>
>>> ---
>>>
>>> Note: Even with this fix included, it is possible to enable gigantic
>>> pages in the fadump kernel. IIUC, gigantic pages were never disabled
>>> for the fadump kernel.
>>>
>>> Currently, gigantic pages are allocated during early boot as long as
>>> the respective hstate is supported by the architecture.
>>>
>>> I will introduce some changes in the generic hugetlb code to allow the
>>> architecture to decide on supporting gigantic pages on the go. Bringing
>>> gigantic page allocation under hugepages_supported() does work for
>>> powerpc but I need verify the impact on other architectures.
>>>
>>> Regarding the Fixes tag: This patch fixes a bug inadvertently introduced
>>> by the commit mentioned under Fixes tag in the commit message. Feel free
>>> to remove the tag if it is unnecessary.
>>>
>>> ---
>>>   arch/powerpc/include/asm/hugetlb.h | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
>>> index 18a3028ac3b6..f294e57663b0 100644
>>> --- a/arch/powerpc/include/asm/hugetlb.h
>>> +++ b/arch/powerpc/include/asm/hugetlb.h
>>> @@ -15,6 +15,15 @@
>>>   
>>>   extern bool hugetlb_disabled;
>>>   
>>> +static inline int hugepages_supported(void)
> I guess we may as well make it's return type as bool.
Yes, fixed in v2:
https://lore.kernel.org/all/20241217074640.1064510-1-sourabhjain@linux.ibm.com/

Thanks again.

- Sourabh Jain