Subject: Re: [PATCH] powerpc/mm: Implemented default_hugepagesz verification
 for powerpc
To: Victor Aoqui <victora@br.ibm.com>, linux-kernel@vger.kernel.org,
        linuxppc-dev@lists.ozlabs.org, aneesh.kumar@linux.vnet.ibm.com,
        mpe@ellerman.id.au
References: <20170703200559.3743-1-victora@br.ibm.com>
Cc: victora@linux.vnet.ibm.com
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Date: Wed, 5 Jul 2017 10:01:26 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.5.1
MIME-Version: 1.0
In-Reply-To: <20170703200559.3743-1-victora@br.ibm.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Message-Id: <4d5950e6-d208-b8d1-ffee-6e54c741c8e0@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1547
Lines: 42

On 07/04/2017 01:35 AM, Victor Aoqui wrote:
> Implemented default hugepage size verification (default_hugepagesz=)
> in order to allow allocation of defined number of pages (hugepages=)
> only for supported hugepage sizes.
> 
> Signed-off-by: Victor Aoqui <victora@br.ibm.com>
> ---
>  arch/powerpc/mm/hugetlbpage.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index a4f33de..464e72e 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -797,6 +797,21 @@ static int __init hugepage_setup_sz(char *str)
>  }
>  __setup("hugepagesz=", hugepage_setup_sz);
> 
> +static int __init default_hugepage_setup_sz(char *str)

The function name should be hugetlb_default_size_setup in
sync with the generic function hugetlb_default_setup for the
same parameter default_hugepagesz.

> +{
> +        unsigned long long size;
> +
> +        size = memparse(str, &str);
> +
> +        if (add_huge_page_size(size) != 0) {

I am little bit confused here. Do we always follow another
'hugepages=' element after 'default_hugepagesz' ? If not,
then we dont have to do 'add_huge_page_size'. But then
that function checks for valid huge page sizes and skips
adding hstate if its already added. So I guess it okay.

> +                hugetlb_bad_size();
> +                pr_err("Invalid default huge page size specified(%llu)\n", size);

Error message should have 'ppc' some where to indicate that
the arch rejected the size not core MM.