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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1E884C433EF for ; Thu, 16 Dec 2021 18:55:49 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8711B6B0071; Thu, 16 Dec 2021 13:55:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7F9966B0073; Thu, 16 Dec 2021 13:55:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 672BC6B0074; Thu, 16 Dec 2021 13:55:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0111.hostedemail.com [216.40.44.111]) by kanga.kvack.org (Postfix) with ESMTP id 52B516B0071 for ; Thu, 16 Dec 2021 13:55:38 -0500 (EST) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 11A7882FFE for ; Thu, 16 Dec 2021 18:55:28 +0000 (UTC) X-FDA: 78924560736.20.7D65625 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf30.hostedemail.com (Postfix) with ESMTP id 80B2580015 for ; Thu, 16 Dec 2021 18:55:27 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id A7BF5B825EC; Thu, 16 Dec 2021 18:55:25 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 41B52C36AE0; Thu, 16 Dec 2021 18:55:22 +0000 (UTC) Date: Thu, 16 Dec 2021 18:55:19 +0000 From: Catalin Marinas To: Christophe Leroy Cc: Nicholas Piggin , "alex@ghiti.fr" , Benjamin Herrenschmidt , Michael Ellerman , Paul Mackerras , Will Deacon , Steve Capper , "akpm@linux-foundation.org" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "linuxppc-dev@lists.ozlabs.org" , Linux ARM Subject: Re: [PATCH v4 07/10] powerpc/mm: Use generic_hugetlb_get_unmapped_area() Message-ID: References: <1639043741.e2zqhea1ix.astroid@bobo.none> <5393b7d1-33e0-2f5c-f2fb-84e6319698c9@csgroup.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <5393b7d1-33e0-2f5c-f2fb-84e6319698c9@csgroup.eu> X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 80B2580015 X-Stat-Signature: 8ywcebqkgh1au9nukdjtkzhp1gen5ufz Authentication-Results: imf30.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none); spf=pass (imf30.hostedemail.com: domain of cmarinas@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=cmarinas@kernel.org X-HE-Tag: 1639680927-133427 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 Thu, Dec 16, 2021 at 05:13:47PM +0000, Christophe Leroy wrote: > Le 09/12/2021 =E0 11:02, Nicholas Piggin a =E9crit=A0: > > Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am: > >> Use the generic version of arch_hugetlb_get_unmapped_area() > >> which is now available at all time. > >> > >> Signed-off-by: Christophe Leroy > >> --- > >> arch/powerpc/include/asm/book3s/64/hugetlb.h | 4 -- > >> arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 55 ----------------= ---- > >> arch/powerpc/mm/hugetlbpage.c | 4 +- > >> 3 files changed, 1 insertion(+), 62 deletions(-) > >> > >> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/pow= erpc/include/asm/book3s/64/hugetlb.h > >> index 12e150e615b7..b37a28f62cf6 100644 > >> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h > >> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h > >> @@ -8,10 +8,6 @@ > >> */ > >> void radix__flush_hugetlb_page(struct vm_area_struct *vma, unsigne= d long vmaddr); > >> void radix__local_flush_hugetlb_page(struct vm_area_struct *vma, u= nsigned long vmaddr); > >> -extern unsigned long > >> -radix__hugetlb_get_unmapped_area(struct file *file, unsigned long a= ddr, > >> - unsigned long len, unsigned long pgoff, > >> - unsigned long flags); > >> =20 > >> extern void radix__huge_ptep_modify_prot_commit(struct vm_area_str= uct *vma, > >> unsigned long addr, pte_t *ptep, > >> diff --git a/arch/powerpc/mm/book3s64/radix_hugetlbpage.c b/arch/pow= erpc/mm/book3s64/radix_hugetlbpage.c > >> index 23d3e08911d3..d2fb776febb4 100644 > >> --- a/arch/powerpc/mm/book3s64/radix_hugetlbpage.c > >> +++ b/arch/powerpc/mm/book3s64/radix_hugetlbpage.c > >> @@ -41,61 +41,6 @@ void radix__flush_hugetlb_tlb_range(struct vm_are= a_struct *vma, unsigned long st > >> radix__flush_tlb_range_psize(vma->vm_mm, start, end, psize); > >> } > >> =20 > >> -/* > >> - * A vairant of hugetlb_get_unmapped_area doing topdown search > >> - * FIXME!! should we do as x86 does or non hugetlb area does ? > >> - * ie, use topdown or not based on mmap_is_legacy check ? > >> - */ > >> -unsigned long > >> -radix__hugetlb_get_unmapped_area(struct file *file, unsigned long a= ddr, > >> - unsigned long len, unsigned long pgoff, > >> - unsigned long flags) > >> -{ > >> - struct mm_struct *mm =3D current->mm; > >> - struct vm_area_struct *vma; > >> - struct hstate *h =3D hstate_file(file); > >> - int fixed =3D (flags & MAP_FIXED); > >> - unsigned long high_limit; > >> - struct vm_unmapped_area_info info; > >> - > >> - high_limit =3D DEFAULT_MAP_WINDOW; > >> - if (addr >=3D high_limit || (fixed && (addr + len > high_limit))) > >> - high_limit =3D TASK_SIZE; > >=20 > > I wonder if generic hugetlb_get_unmapped_area needs to have the > > arch_get_mmap_end() added. > >=20 > > arm64 has arch_get_mmap_end() and !HAVE_ARCH_HUGETLB_UNMAPPED_AREA so > > it looks like it has broken large address hint logic for hugetlbfs > > mappings? x86-64 defines their own and does the same hinting for > > normal and hugetlbfs mmap. > >=20 > > If we had that and defied arch_get_mmap_end(), then this patch should > > work. > >=20 >=20 > As far as I can see, hugetlb_get_unmapped_area() variants used to be=20 > very similar to get_unmapped_area() until commit 1be7107fbe18 ("mm:=20 > larger stack guard gap, between vmas") and commit f6795053dac8 ("mm:=20 > mmap: Allow for "high" userspace addresses") >=20 > I see no reason why those changes couldn't apply to=20 > hugetlb_get_unmapped_area() as well. >=20 > Need to know what ARM64 think about it thought. Will, Catalin, any opin= ion ? I think we should have fixed hugetlb_get_unmapped_area() as well when we added support for 52-bit VA. The reason for commit f6795053dac8 was to prevent normal mmap() from returning addresses above 48-bit by default as some user-space had hard assumptions about this. It's a slight ABI change if you do this for hugetlb_get_unmapped_area() but I doubt anyone would notice. It's more likely that the current behaviour would cause issues, so I'd rather have them consistent. --=20 Catalin