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 644AFCD4F32 for ; Fri, 22 Sep 2023 08:41:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F04A76B029E; Fri, 22 Sep 2023 04:41:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EB4696B029F; Fri, 22 Sep 2023 04:41:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D7C2C6B02A0; Fri, 22 Sep 2023 04:41:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id C79206B029E for ; Fri, 22 Sep 2023 04:41:09 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 8F01C120FAA for ; Fri, 22 Sep 2023 08:41:09 +0000 (UTC) X-FDA: 81263588658.29.E91DDD2 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf19.hostedemail.com (Postfix) with ESMTP id C311B1A0024 for ; Fri, 22 Sep 2023 08:41:07 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf19.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695372067; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Rxdkhj4+nl5rPYVFayYzWhQP2DR7MOQ2TllvwTypxVA=; b=XDESGbRPg87M2ukg/U8FKvOMVh44pCLRjSbTRsY04jFlUVKHpBl1TWk6J3I8JrUAB5OoiU 9Hg9EoXBG8Wto5DX6UNfWZykUfH2l6FijTUbQy+OK19LRCT/od/7Qo2P7USlmjQAlEht4r QXnQtTXpPsmj1XgakeZcPx/jm8CkSnY= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf19.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695372067; a=rsa-sha256; cv=none; b=Uxv+FnfOzgr5e7sF8PNF35W4EGKHfyoiDPwnpfw4vJCfBkPD9AC3IjI6X6OIK7jdskhMLQ PfmYWTLm0Y3YuMP2WLTL3xuvYV86MRGU146AAjhgNgCOumDSAG/kwnZYFS4XLT3AA0KBLO 4eWWBP/U/yiM3E9WQp9VoFrod7LwoSQ= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id ED220DA7; Fri, 22 Sep 2023 01:41:43 -0700 (PDT) Received: from [10.57.65.11] (unknown [10.57.65.11]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4E23B3F67D; Fri, 22 Sep 2023 01:41:02 -0700 (PDT) Message-ID: <871d277e-6c4e-418a-9574-2e9117646ca2@arm.com> Date: Fri, 22 Sep 2023 09:41:00 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 2/8] powerpc: hugetlb: Convert set_huge_pte_at() to take vma Content-Language: en-GB To: Christophe Leroy , Catalin Marinas , Will Deacon , "James E.J. Bottomley" , Helge Deller , Nicholas Piggin , Paul Walmsley , Palmer Dabbelt , Albert Ou , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Sven Schnelle , Gerald Schaefer , "David S. Miller" , Arnd Bergmann , Mike Kravetz , Muchun Song , SeongJae Park , Andrew Morton , Uladzislau Rezki , Christoph Hellwig , Lorenzo Stoakes , Anshuman Khandual , Peter Xu , Axel Rasmussen , Qi Zheng Cc: "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-parisc@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , "linux-riscv@lists.infradead.org" , "linux-s390@vger.kernel.org" , "sparclinux@vger.kernel.org" , "linux-mm@kvack.org" , "stable@vger.kernel.org" References: <20230921162007.1630149-1-ryan.roberts@arm.com> <20230921162007.1630149-3-ryan.roberts@arm.com> <0f2dd31a-a361-24ba-154d-4e614c0eff73@csgroup.eu> <597e7496-ef99-4505-88ee-91701af02835@arm.com> <5ee382da-0ed0-9fb1-7640-35ef77c195fc@csgroup.eu> From: Ryan Roberts In-Reply-To: <5ee382da-0ed0-9fb1-7640-35ef77c195fc@csgroup.eu> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: C311B1A0024 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: kfrzhp19rhxp3zj9jyb9eynt6mojz3oe X-HE-Tag: 1695372067-529366 X-HE-Meta: U2FsdGVkX18ftQ7OGQtEtXN9OqFRI23Kxfilcvso7/v95zOWSdkntn8lKh6H7UhEgdRIzkSrvcQPNyoCT0ctt959pqp4zZA5o3WgrE2sCu/39dLkya9cumpI3sOq0HklCi2MDZhhM5C3CINn2l4zFN++PYCYijTuVpdWI/llvIBkZCS3vhHMMsq+kfbElZnydLWZzdZ3ks9wVlGSb64tult18ezdtMHhfNl3u2RJOkq3FNo97lg9FmbOuEkn+NfQeYXWAT7BUdqa5XIzhzRFG1LsWbYHFgOy1OlodkpuI9dFDsiIkt7Qto4LykSD7ncltD52xO+UDvYlRZ4XSUeVkmEQCvM+1kCgBqRWY+cXvaGdbp6l+Fsy5wZkzezQ6TKF2lHs845Qq0q/Srgfs91yWCVnc0ETrUCBQhcr+bVvCYAyt/AUJZQz1fftxIjEpVEvJmBK9mFKWVNPfFRr4s9IjOrYBLpyIQsSxYr3Ufi/zObLryR0eDjYP3PfnnATxALMa+sMyYA32KCjEmXiQaOcpY3mZX89Djb/EZhP01D8rdnkQOg6m5L8U9NNosF95aaXdf+MhAILmra8Tyxx4oO1ViWcgx20EOx/zUQd5tDAfWOV336KPqur/msvre6C9MQcAPcXzVqtATiNlFiQuT+HSYvfAc8NAtWwOQ0ij1QhT0dXZThj6pPf3r+3OhcQoAMlZqyhPB+Rbjvg+KJ2GtWPoeK1LJgkSbq3so6fXTmWFTELEoH87mPAmcc0a6P71aLKv8J22ka6rUcGr+2pdHgOZ59wMQLr+wg62FNQ5Xyo1hMptWpHvVd0QnHOg3q4rk3HTGE+/WuzNIJNmV4SGpbEZWsPusOyUGxkZNF0rmIPSA/NSva3lM9pJWpTOB1UmuJzDhP38wykRKkJzrF+F9UZ71y9pXpBUGWcRL0mz8XSiJo39v8s/emdTa3jgrty602EtWhafdmYrclrn7mXn5J S7Id8BWB qVysXGf45Hvvj5ZQsdy9GQVozOOsMuvKCiVK2vFm+9hfng71fOfGUl2ame1u14mrRIZ2IB5HG5IpmusHyVetv7VS6DycMd8vafUkYWUg6Ax1sY18iTVBKqlkmrHYi9JPpcsZuFUYAg6HkyvOZDwVKANGYOcztz3wgavniRp6lTYgsbG2nhtAXdX1Vf1uWzPnI1y2SdaQjjvkK1HmrYoEy0ggPuJrKxTW2+ymhoXcFsklz1FLfMj11jgUGZSYMy4P3isWezXZMis5bsD242S0uNmFWIaGktjWzEKAChWUwAr81c/FNaUtKcDIBrovy0TWReWYhdkmec/+M5WCYFzgTL1GObYTDP1z8YYRR 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 22/09/2023 09:10, Christophe Leroy wrote: > > > Le 22/09/2023 à 09:33, Ryan Roberts a écrit : >> On 22/09/2023 07:56, Christophe Leroy wrote: >>> >>> >>> Le 21/09/2023 à 18:20, Ryan Roberts a écrit : >>>> In order to fix a bug, arm64 needs access to the vma inside it's >>>> implementation of set_huge_pte_at(). Provide for this by converting the >>>> mm parameter to be a vma. Any implementations that require the mm can >>>> access it via vma->vm_mm. >>>> >>>> This commit makes the required powerpc modifications. Separate commits >>>> update the other arches and core code, before the actual bug is fixed in >>>> arm64. >>>> >>>> No behavioral changes intended. >>>> >>>> Signed-off-by: Ryan Roberts >>>> --- >>>> arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h | 3 ++- >>>> arch/powerpc/mm/book3s64/hugetlbpage.c | 2 +- >>>> arch/powerpc/mm/book3s64/radix_hugetlbpage.c | 2 +- >>>> arch/powerpc/mm/nohash/8xx.c | 2 +- >>>> arch/powerpc/mm/pgtable.c | 7 ++++++- >>>> 5 files changed, 11 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h >>>> index de092b04ee1a..fff8cd726bc7 100644 >>>> --- a/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h >>>> +++ b/arch/powerpc/include/asm/nohash/32/hugetlb-8xx.h >>>> @@ -46,7 +46,8 @@ static inline int check_and_get_huge_psize(int shift) >>>> } >>>> >>>> #define __HAVE_ARCH_HUGE_SET_HUGE_PTE_AT >>>> -void set_huge_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte); >>>> +void set_huge_pte_at(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep, pte_t pte); >>>> +void __set_huge_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte); >>> >>> Don't add the burden of an additional function, see below >>> >>>> >>>> #define __HAVE_ARCH_HUGE_PTE_CLEAR >>>> static inline void huge_pte_clear(struct mm_struct *mm, unsigned long addr, >>>> diff --git a/arch/powerpc/mm/book3s64/hugetlbpage.c b/arch/powerpc/mm/book3s64/hugetlbpage.c >>>> index 3bc0eb21b2a0..ae7fd7c90eb8 100644 >>>> --- a/arch/powerpc/mm/book3s64/hugetlbpage.c >>>> +++ b/arch/powerpc/mm/book3s64/hugetlbpage.c >>>> @@ -147,7 +147,7 @@ void huge_ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr >>>> if (radix_enabled()) >>>> return radix__huge_ptep_modify_prot_commit(vma, addr, ptep, >>>> old_pte, pte); >>>> - set_huge_pte_at(vma->vm_mm, addr, ptep, pte); >>>> + set_huge_pte_at(vma, addr, ptep, pte); >>>> } >>>> >>>> void __init hugetlbpage_init_defaultsize(void) >>>> diff --git a/arch/powerpc/mm/book3s64/radix_hugetlbpage.c b/arch/powerpc/mm/book3s64/radix_hugetlbpage.c >>>> index 17075c78d4bc..7cd40a334c3a 100644 >>>> --- a/arch/powerpc/mm/book3s64/radix_hugetlbpage.c >>>> +++ b/arch/powerpc/mm/book3s64/radix_hugetlbpage.c >>>> @@ -58,5 +58,5 @@ void radix__huge_ptep_modify_prot_commit(struct vm_area_struct *vma, >>>> atomic_read(&mm->context.copros) > 0) >>>> radix__flush_hugetlb_page(vma, addr); >>>> >>>> - set_huge_pte_at(vma->vm_mm, addr, ptep, pte); >>>> + set_huge_pte_at(vma, addr, ptep, pte); >>>> } >>>> diff --git a/arch/powerpc/mm/nohash/8xx.c b/arch/powerpc/mm/nohash/8xx.c >>>> index dbbfe897455d..650a7a8496b6 100644 >>>> --- a/arch/powerpc/mm/nohash/8xx.c >>>> +++ b/arch/powerpc/mm/nohash/8xx.c >>>> @@ -91,7 +91,7 @@ static int __ref __early_map_kernel_hugepage(unsigned long va, phys_addr_t pa, >>>> if (new && WARN_ON(pte_present(*ptep) && pgprot_val(prot))) >>>> return -EINVAL; >>>> >>>> - set_huge_pte_at(&init_mm, va, ptep, pte_mkhuge(pfn_pte(pa >> PAGE_SHIFT, prot))); >>>> + __set_huge_pte_at(&init_mm, va, ptep, pte_mkhuge(pfn_pte(pa >> PAGE_SHIFT, prot))); >>> >>> Call set_huge_pte_at() with a NULL vma instead. >> >> I'm happy to take your proposed approach if that's your preference. Another >> option is to use a dummy VMA, as I have done in the core code, for the one call >> site that calls set_huge_pte_at() with init_mm: >> >> struct vm_area_struct vma = TLB_FLUSH_VMA(&init_mm, 0); >> >> This is an existing macro that creates a dummy vma with vma->vm_mm filled in. >> Then I pass &vma to the function. > > I don't like that, I prefer the solution I proposed. We already have a > couple places where powerpc do things based on whether vma is NULL or not. > >> >> Or yet another option would be to keep the mm param as is in set_huge_pte_at(), >> and add a size param to the function. But then all call sites have the burden of >> figuring out the size of the huge pte (although I think most know already). > > Indeed. > > arch_make_huge_pte() used to take a vma until commit 79c1c594f49a > ("mm/hugetlb: change parameters of arch_make_huge_pte()"). > > Should we try and have the same approach ? Or is it irrelevant ? See [1]; I'm going to rework to pass mm + size parameter since the current approach will break riscv. [1] https://lore.kernel.org/linux-arm-kernel/fc85f58e-e8ed-4b24-a3e5-d6288156595e@arm.com/ > > Christophe > >> >> Thanks, >> Ryan >> >>> >>>> >>>> return 0; >>>> } >>>> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c >>>> index 3f86fd217690..9cbcb561a4d8 100644 >>>> --- a/arch/powerpc/mm/pgtable.c >>>> +++ b/arch/powerpc/mm/pgtable.c >>>> @@ -288,7 +288,7 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma, >>>> } >>>> >>>> #if defined(CONFIG_PPC_8xx) >>>> -void set_huge_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte) >>>> +void __set_huge_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_t pte) >>> >>> Keep it as set_huge_pte_at() with vma argument. >>> >>>> { >>>> pmd_t *pmd = pmd_off(mm, addr); >>> >>> Change to: >>> >>> pmd_t *pmd = vma ? pmd_off(vma->vm_mm, addr) : pmd_off_k(addr); >>> >>>> pte_basic_t val; >>>> @@ -310,6 +310,11 @@ void set_huge_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep, pte_ >>>> for (i = 0; i < num; i++, entry++, val += SZ_4K) >>>> *entry = val; >>>> } >>>> + >>>> +void set_huge_pte_at(struct vm_area_struct *vma, unsigned long addr, pte_t *ptep, pte_t pte) >>>> +{ >>>> + __set_huge_pte_at(vma->vm_mm, addr, ptep, pte); >>>> +} >>> >>> Remove this burden. >>> >>>> #endif >>>> #endif /* CONFIG_HUGETLB_PAGE */ >>>> >>> >>> >>> Christophe >>