Re: [RFC PATCH 3/3] powerpc/mm/hugetlb: Don't enable HugeTLB if we don't have a page table cache

[Michael Ellerman <mpe@ellerman.id.au>]

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> This makes sure we don't enable HugeTLB if the cache is not configured.
> I am still not sure about this. IMHO hugetlb support should be a hardware
> support derivative and any cache allocation failure should be handled as I did
> in the earlier patch. But then if we were not able to create hugetlb page table
> cache, we can as well declare hugetlb support disabled thereby avoiding calling
> into allocation routines.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  arch/powerpc/mm/hugetlbpage.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index ee16a3fb788a..4bf8bc659cc7 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -602,6 +602,7 @@ __setup("hugepagesz=", hugepage_setup_sz);
>  static int __init hugetlbpage_init(void)
>  {
>  	int psize;
> +	bool configured = false;

Where's my reverse Christmas tree! :)

>  	if (hugetlb_disabled) {
>  		pr_info("HugeTLB support is disabled!\n");
> @@ -651,10 +652,16 @@ static int __init hugetlbpage_init(void)
>  			pgtable_cache_add(pdshift - shift);
>  		else if (IS_ENABLED(CONFIG_PPC_FSL_BOOK3E) || IS_ENABLED(CONFIG_PPC_8xx))
>  			pgtable_cache_add(PTE_T_ORDER);
> +
> +		if (!configured)
> +			configured = true;

I'd just not worry about the if.

>  	}
>  
> -	if (IS_ENABLED(CONFIG_HUGETLB_PAGE_SIZE_VARIABLE))
> -		hugetlbpage_init_default();
> +	if (configured) {
> +		if (IS_ENABLED(CONFIG_HUGETLB_PAGE_SIZE_VARIABLE))
> +			hugetlbpage_init_default();
> +	} else
> +		pr_info("Disabling HugeTLB");

We're not actually doing anything to disable it in the
CONFIG_HUGETLB_PAGE_SIZE_VARIABLE=n case, but I guess the print is still
correct because we didn't enable a size in the for loop above?

Can we make it a bit more explicit? Maybe like:

  "Disabling HugeTLB, no usable page sizes found."

??

cheers