From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 33BBDC4332D for ; Thu, 19 Mar 2020 07:01:10 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B616A20714 for ; Thu, 19 Mar 2020 07:01:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=c-s.fr header.i=@c-s.fr header.b="kSCCxuWg" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B616A20714 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=c-s.fr Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 19D676B0003; Thu, 19 Mar 2020 03:01:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 14EA86B0005; Thu, 19 Mar 2020 03:01:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 015D26B0006; Thu, 19 Mar 2020 03:01:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0108.hostedemail.com [216.40.44.108]) by kanga.kvack.org (Postfix) with ESMTP id DB4CD6B0003 for ; Thu, 19 Mar 2020 03:01:08 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 864515010 for ; Thu, 19 Mar 2020 07:01:08 +0000 (UTC) X-FDA: 76611215016.05.woman63_26f1052653b06 X-HE-Tag: woman63_26f1052653b06 X-Filterd-Recvd-Size: 10587 Received: from pegase1.c-s.fr (pegase1.c-s.fr [93.17.236.30]) by imf04.hostedemail.com (Postfix) with ESMTP for ; Thu, 19 Mar 2020 07:01:07 +0000 (UTC) Received: from localhost (mailhub1-int [192.168.12.234]) by localhost (Postfix) with ESMTP id 48jd9D5Wspz9v1Md; Thu, 19 Mar 2020 08:01:04 +0100 (CET) Authentication-Results: localhost; dkim=pass reason="1024-bit key; insecure key" header.d=c-s.fr header.i=@c-s.fr header.b=kSCCxuWg; dkim-adsp=pass; dkim-atps=neutral X-Virus-Scanned: Debian amavisd-new at c-s.fr Received: from pegase1.c-s.fr ([192.168.12.234]) by localhost (pegase1.c-s.fr [192.168.12.234]) (amavisd-new, port 10024) with ESMTP id a91KdXxJSw9z; Thu, 19 Mar 2020 08:01:04 +0100 (CET) Received: from messagerie.si.c-s.fr (messagerie.si.c-s.fr [192.168.25.192]) by pegase1.c-s.fr (Postfix) with ESMTP id 48jd9D4MMFz9v1Mc; Thu, 19 Mar 2020 08:01:04 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=c-s.fr; s=mail; t=1584601264; bh=f1YwOhdxX26Y+wBuzZ6KcmY62u5y/+8Hvgm0wH6ueYs=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From; b=kSCCxuWg89EGpfCGJFvQQEKaoHe/kCd5TGb/eJd9YNW4o9nMza+I2DetAh60y0OYl jNWoevB3fgK3oMJ2kFYnvfR61+Z0w2UWeOnoIyGuiPFaUDv3CG1pmdSv9t5BVLTXov N7ub0KXLdsT/u4I32YAoSgYf0fhvDN90dndC0kTk= Received: from localhost (localhost [127.0.0.1]) by messagerie.si.c-s.fr (Postfix) with ESMTP id 784F58B769; Thu, 19 Mar 2020 08:01:05 +0100 (CET) X-Virus-Scanned: amavisd-new at c-s.fr Received: from messagerie.si.c-s.fr ([127.0.0.1]) by localhost (messagerie.si.c-s.fr [127.0.0.1]) (amavisd-new, port 10023) with ESMTP id MWrtuWx18n7g; Thu, 19 Mar 2020 08:01:05 +0100 (CET) Received: from [192.168.4.90] (unknown [192.168.4.90]) by messagerie.si.c-s.fr (Postfix) with ESMTP id DFB5B8B798; Thu, 19 Mar 2020 08:01:03 +0100 (CET) Subject: Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size To: Mike Kravetz , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, sparclinux@vger.kernel.org, linux-doc@vger.kernel.org Cc: Albert Ou , Andrew Morton , Vasily Gorbik , Jonathan Corbet , Catalin Marinas , Dave Hansen , Heiko Carstens , Christian Borntraeger , Ingo Molnar , Palmer Dabbelt , Paul Walmsley , Paul Mackerras , Thomas Gleixner , Longpeng , Will Deacon , "David S . Miller" References: <20200318220634.32100-1-mike.kravetz@oracle.com> <20200318220634.32100-2-mike.kravetz@oracle.com> From: Christophe Leroy Message-ID: Date: Thu, 19 Mar 2020 08:00:59 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200318220634.32100-2-mike.kravetz@oracle.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Le 18/03/2020 =C3=A0 23:06, Mike Kravetz a =C3=A9crit=C2=A0: > The architecture independent routine hugetlb_default_setup sets up > the default huge pages size. It has no way to verify if the passed > value is valid, so it accepts it and attempts to validate at a later > time. This requires undocumented cooperation between the arch specific > and arch independent code. >=20 > For architectures that support more than one huge page size, provide > a routine arch_hugetlb_valid_size to validate a huge page size. > hugetlb_default_setup can use this to validate passed values. >=20 > arch_hugetlb_valid_size will also be used in a subsequent patch to > move processing of the "hugepagesz=3D" in arch specific code to a commo= n > routine in arch independent code. >=20 > Signed-off-by: Mike Kravetz > --- > arch/arm64/include/asm/hugetlb.h | 2 ++ > arch/arm64/mm/hugetlbpage.c | 19 ++++++++++++++----- > arch/powerpc/include/asm/hugetlb.h | 3 +++ > arch/powerpc/mm/hugetlbpage.c | 20 +++++++++++++------- > arch/riscv/include/asm/hugetlb.h | 3 +++ > arch/riscv/mm/hugetlbpage.c | 28 ++++++++++++++++++---------- > arch/s390/include/asm/hugetlb.h | 3 +++ > arch/s390/mm/hugetlbpage.c | 18 +++++++++++++----- > arch/sparc/include/asm/hugetlb.h | 3 +++ > arch/sparc/mm/init_64.c | 23 ++++++++++++++++------- > arch/x86/include/asm/hugetlb.h | 3 +++ > arch/x86/mm/hugetlbpage.c | 21 +++++++++++++++------ > include/linux/hugetlb.h | 7 +++++++ > mm/hugetlb.c | 16 +++++++++++++--- > 14 files changed, 126 insertions(+), 43 deletions(-) >=20 [snip] > diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/= asm/hugetlb.h > index bd6504c28c2f..3b5939016955 100644 > --- a/arch/powerpc/include/asm/hugetlb.h > +++ b/arch/powerpc/include/asm/hugetlb.h > @@ -64,6 +64,9 @@ static inline void arch_clear_hugepage_flags(struct p= age *page) > { > } > =20 > +#define arch_hugetlb_valid_size arch_hugetlb_valid_size > +extern bool __init arch_hugetlb_valid_size(unsigned long long size); Don't add 'extern' keyword, it is irrelevant for a function declaration. checkpatch --strict doesn't like it either=20 (https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/12318= //artifact/linux/checkpatch.log) > + > #include > =20 > #else /* ! CONFIG_HUGETLB_PAGE */ > diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpag= e.c > index 33b3461d91e8..b78f660252f3 100644 > --- a/arch/powerpc/mm/hugetlbpage.c > +++ b/arch/powerpc/mm/hugetlbpage.c > @@ -558,7 +558,7 @@ unsigned long vma_mmu_pagesize(struct vm_area_struc= t *vma) > return vma_kernel_pagesize(vma); > } > =20 > -static int __init add_huge_page_size(unsigned long long size) > +bool __init arch_hugetlb_valid_size(unsigned long long size) > { > int shift =3D __ffs(size); > int mmu_psize; > @@ -566,20 +566,26 @@ static int __init add_huge_page_size(unsigned lon= g long size) > /* Check that it is a page size supported by the hardware and > * that it fits within pagetable and slice limits. */ > if (size <=3D PAGE_SIZE || !is_power_of_2(size)) > - return -EINVAL; > + return false; > =20 > mmu_psize =3D check_and_get_huge_psize(shift); > if (mmu_psize < 0) > - return -EINVAL; > + return false; > =20 > BUG_ON(mmu_psize_defs[mmu_psize].shift !=3D shift); > =20 > - /* Return if huge page size has already been setup */ > - if (size_to_hstate(size)) > - return 0; > + return true; > +} > =20 > - hugetlb_add_hstate(shift - PAGE_SHIFT); > +static int __init add_huge_page_size(unsigned long long size) > +{ > + int shift =3D __ffs(size); > + > + if (!arch_hugetlb_valid_size(size)) > + return -EINVAL; > =20 > + if (!size_to_hstate(size)) > + hugetlb_add_hstate(shift - PAGE_SHIFT); > return 0; > } > =20 [snip] > diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c > index 5bfd5aef5378..51e6208fdeec 100644 > --- a/arch/x86/mm/hugetlbpage.c > +++ b/arch/x86/mm/hugetlbpage.c > @@ -181,16 +181,25 @@ hugetlb_get_unmapped_area(struct file *file, unsi= gned long addr, > #endif /* CONFIG_HUGETLB_PAGE */ > =20 > #ifdef CONFIG_X86_64 > +bool __init arch_hugetlb_valid_size(unsigned long long size) > +{ > + if (size =3D=3D PMD_SIZE) > + return true; > + else if (size =3D=3D PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES)) > + return true; > + else > + return false; > +} > + > static __init int setup_hugepagesz(char *opt) > { > - unsigned long ps =3D memparse(opt, &opt); > - if (ps =3D=3D PMD_SIZE) { > - hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT); > - } else if (ps =3D=3D PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES)) { > - hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT); > + unsigned long long ps =3D memparse(opt, &opt); > + > + if (arch_hugetlb_valid_size(ps)) { > + hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT); > } else { > hugetlb_bad_size(); > - printk(KERN_ERR "hugepagesz: Unsupported page size %lu M\n", > + printk(KERN_ERR "hugepagesz: Unsupported page size %llu M\n", > ps >> 20); Nowadays we use pr_err() instead of printk. It would also likely allow you to have everything fit on a single line. > return 0; > } > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h > index b831e9fa1a26..33343eb980d0 100644 > --- a/include/linux/hugetlb.h > +++ b/include/linux/hugetlb.h > @@ -678,6 +678,13 @@ static inline spinlock_t *huge_pte_lockptr(struct = hstate *h, > return &mm->page_table_lock; > } > =20 > +#ifndef arch_hugetlb_valid_size > +static inline bool arch_hugetlb_valid_size(unsigned long long size) > +{ > + return (size =3D=3D HPAGE_SIZE); Not sure the ( ) are necessary. > +} > +#endif > + > #ifndef hugepages_supported > /* > * Some platform decide whether they support huge pages at boot > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index d8ebd876871d..2f99359b93af 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -3224,12 +3224,22 @@ static int __init hugetlb_nrpages_setup(char *s= ) > } > __setup("hugepages=3D", hugetlb_nrpages_setup); > =20 > -static int __init hugetlb_default_setup(char *s) > +static int __init default_hugepagesz_setup(char *s) > { > - default_hstate_size =3D memparse(s, &s); > + unsigned long long size; Why unsigned long long ? default_hstate_size is long. I can't imagine 32 bits platforms having a hugepage with a 64 bits size. > + char *saved_s =3D s; > + > + size =3D memparse(s, &s); The updated s is not reused after that so you can pass NULL instead of=20 &s and then you don't need the saved_s. > + > + if (!arch_hugetlb_valid_size(size)) { > + pr_err("HugeTLB: unsupported default_hugepagesz %s\n", saved_s); > + return 0; > + } > + > + default_hstate_size =3D size; > return 1; > } > -__setup("default_hugepagesz=3D", hugetlb_default_setup); > +__setup("default_hugepagesz=3D", default_hugepagesz_setup); > =20 > static unsigned int cpuset_mems_nr(unsigned int *array) > { >=20 Christophe