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 A653CC4332B for ; Thu, 19 Mar 2020 18:18:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5104020724 for ; Thu, 19 Mar 2020 18:18:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=oracle.com header.i=@oracle.com header.b="rmpVyB3d" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5104020724 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=oracle.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id DD2D56B000A; Thu, 19 Mar 2020 14:18:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D5C706B000C; Thu, 19 Mar 2020 14:18:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BFD346B000D; Thu, 19 Mar 2020 14:18:52 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0199.hostedemail.com [216.40.44.199]) by kanga.kvack.org (Postfix) with ESMTP id 9E7AD6B000A for ; Thu, 19 Mar 2020 14:18:52 -0400 (EDT) Received: from smtpin23.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 5957155FA4 for ; Thu, 19 Mar 2020 18:18:52 +0000 (UTC) X-FDA: 76612922904.23.drug07_844fcf174415d X-HE-Tag: drug07_844fcf174415d X-Filterd-Recvd-Size: 12436 Received: from aserp2120.oracle.com (aserp2120.oracle.com [141.146.126.78]) by imf48.hostedemail.com (Postfix) with ESMTP for ; Thu, 19 Mar 2020 18:18:25 +0000 (UTC) Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 02JI8wHq080123; Thu, 19 Mar 2020 18:17:55 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2020-01-29; bh=HXvLR8/3np/VEn4lQcPgfkQDXtCeUG1sWXEv4yIwMjg=; b=rmpVyB3d0/cWvHtU9RvF5VWb6i7rdQSOdwNYMXZ5oeoLYOUo/sWN/JFVznP4TARCd8/j IOZcupF92YGaCAal0s1483+WIxGL9ohqg+S6vn4mv0ZLtI6i+73jqJUxy/++kh21IAx6 psTm0iugjrftvG1vtreP8z/8AhtmTionvoNYc5FGURy0BlrOXCHHWs6w8IOKxIXmgPsA tqY9g4VPq5KdtvZmjA7fhcxui2F7jvT92/u+ESkjswUWFiQ6i3uOitEqbhB1XvsjWMBY YJa5yi8fzQ+JWcuU6zZ3KN4HO7sVh44X2X+NQ5wlD/8IIDNn78qOXnLf6dnarU3i3pwi Ew== Received: from userp3030.oracle.com (userp3030.oracle.com [156.151.31.80]) by aserp2120.oracle.com with ESMTP id 2yrq7m9tpb-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 19 Mar 2020 18:17:55 +0000 Received: from pps.filterd (userp3030.oracle.com [127.0.0.1]) by userp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 02JIGdv1032600; Thu, 19 Mar 2020 18:17:54 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userp3030.oracle.com with ESMTP id 2ys8rmsrj4-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 19 Mar 2020 18:17:54 +0000 Received: from abhmp0007.oracle.com (abhmp0007.oracle.com [141.146.116.13]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 02JIHoi9022986; Thu, 19 Mar 2020 18:17:50 GMT Received: from [192.168.1.206] (/71.63.128.209) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 19 Mar 2020 11:17:49 -0700 Subject: Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size To: Christophe Leroy , 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: Mike Kravetz Message-ID: Date: Thu, 19 Mar 2020 11:17:47 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9565 signatures=668685 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 phishscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999 adultscore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2003020000 definitions=main-2003190077 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9565 signatures=668685 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 lowpriorityscore=0 suspectscore=0 adultscore=0 bulkscore=0 mlxlogscore=999 priorityscore=1501 clxscore=1015 malwarescore=0 mlxscore=0 phishscore=0 impostorscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2003020000 definitions=main-2003190076 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: On 3/19/20 12:00 AM, Christophe Leroy wrote: >=20 > Le 18/03/2020 =C3=A0 23:06, Mike Kravetz a =C3=A9crit : >> 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 specifi= c >> and arch independent code. >> >> 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. >> >> 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 comm= on >> routine in arch independent code. >> >> 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] >=20 >> 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 = page *page) >> { >> } >> +#define arch_hugetlb_valid_size arch_hugetlb_valid_size >> +extern bool __init arch_hugetlb_valid_size(unsigned long long size); >=20 > Don't add 'extern' keyword, it is irrelevant for a function declaration= . >=20 Will do. One of the other arch's did this and I got into a bad habit. > checkpatch --strict doesn't like it either (https://openpower.xyz/job/s= nowpatch/job/snowpatch-linux-checkpatch/12318//artifact/linux/checkpatch.= log) >=20 >> + >> #include >> #else /* ! CONFIG_HUGETLB_PAGE */ >> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpa= ge.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_stru= ct *vma) >> return vma_kernel_pagesize(vma); >> } >> -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 lo= ng 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; >> mmu_psize =3D check_and_get_huge_psize(shift); >> if (mmu_psize < 0) >> - return -EINVAL; >> + return false; >> BUG_ON(mmu_psize_defs[mmu_psize].shift !=3D shift); >> - /* Return if huge page size has already been setup */ >> - if (size_to_hstate(size)) >> - return 0; >> + return true; >> +} >> - 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; >> + if (!size_to_hstate(size)) >> + hugetlb_add_hstate(shift - PAGE_SHIFT); >> return 0; >> } >> =20 >=20 > [snip] >=20 >> 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, uns= igned long addr, >> #endif /* CONFIG_HUGETLB_PAGE */ >> #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); >=20 > Nowadays we use pr_err() instead of printk. >=20 > It would also likely allow you to have everything fit on a single line. I may just leave this 'as is' as it will be removed in a later patch. >> 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; >> } >> +#ifndef arch_hugetlb_valid_size >> +static inline bool arch_hugetlb_valid_size(unsigned long long size) >> +{ >> + return (size =3D=3D HPAGE_SIZE); >=20 > Not sure the ( ) are necessary. Likely not. I will look at removing. >=20 >> +} >> +#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); >> -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; >=20 > Why unsigned long long ? >=20 > default_hstate_size is long. Only because memparse is defined as unsigned long long. I actually took this from the existing powerpc hugetlb setup code. There are no compiler warnings/issues assigning unsigned long long to long on 64 bit builds. Thought there would be on 32 bit platformes. That was also the reason for making the argument to arch_hugetlb_valid_si= ze be unsigned long long. So that it would match the type from memparse. I suppose making these unsigned long and casting would be OK based on the expected sizes. >=20 > I can't imagine 32 bits platforms having a hugepage with a 64 bits size= . >=20 >> + char *saved_s =3D s; >> + >> + size =3D memparse(s, &s); >=20 > The updated s is not reused after that so you can pass NULL instead of = &s and then you don't need the saved_s. >=20 Thanks for this and all the comments. I will incorporate in v2. --=20 Mike Kravetz