Re: [PATCH] mm/hugetlb: bring gigantic page allocation under hugepages_supported()

[Sourabh Jain <sourabhjain@linux.ibm.com>]

Hello Gerald,

On 22/01/25 19:36, Gerald Schaefer wrote:
> On Tue, 21 Jan 2025 20:34:19 +0530
> Sourabh Jain <sourabhjain@linux.ibm.com> wrote:
>
>> Despite having kernel arguments to enable gigantic hugepages, this
>> provides a way for the architecture to disable gigantic hugepages on the
>> fly, similar to what we do for hugepages.
>>
>> Components like fadump (PowerPC-specific) need this functionality to
>> disable gigantic hugepages when the kernel is booted solely to collect
>> the kernel core dump.
>>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Borislav Petkov <bp@alien8.de>
>> Cc: Heiko Carstens <hca@linux.ibm.com>
>> Cc: Vasily Gorbik <gor@linux.ibm.com>
>> Cc: Muchun Song <muchun.song@linux.dev>
>> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: linux-mm@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>
>> To evaluate the impact of this change on architectures other than
>> PowerPC, I did the following analysis:
>>
>> For architectures where hugepages_supported() is not redefined, it
>> depends on HPAGE_SHIFT, which is found to be a constant. It is mostly
>> initialized to PMD_SHIFT.
>>
>> Architecture : HPAGE_SHIFT initialized with
>>
>> ARC: PMD_SHIFT (constant)
>> ARM: PMD_SHIFT (constant)
>> ARM64: PMD_SHIFT (constant)
>> Hexagon: 22 (constant)
>> LoongArch: (PAGE_SHIFT + PAGE_SHIFT - 3) (appears to be constant)
>> MIPS: (PAGE_SHIFT + PAGE_SHIFT - 3) (appears to be constant)
>> PARISC: PMD_SHIFT (appears to be constant)
>> RISC-V: PMD_SHIFT (constant)
>> SH: 16 | 18 | 20 | 22 | 26 (constant)
>> SPARC: 23 (constant)
>>
>> So seems like this change shouldn't have any impact on above
>> architectures.
>>
>> On the S390 and X86 architectures, hugepages_supported() is redefined,
>> and I am uncertain at what point it is safe to call
>> hugepages_supported().
> For s390, hugepages_supported() checks EDAT1 machine flag, which is
> initialized long before any initcalls. So it is safe to be called
> here.
Thanks for the info.
>
> My common code hugetlb skills got a little rusty, but shouldn't
> arch_hugetlb_valid_size() already prevent getting here for gigantic
> hugepages, in case they are not supported? And could you not use
> that for your purpose?

Yes, handling this in arch_hugetlb_valid_size is even better. That way,
we can avoid initializing data structures to hold hstate, which is not
required anyway.

Thanks for the review and suggestion. I will handle this in the
architecture-specific code.

- Sourabh Jain