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 D14A7C35274 for ; Fri, 15 Dec 2023 14:05:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 54D906B047B; Fri, 15 Dec 2023 09:05:37 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4D66B6B047C; Fri, 15 Dec 2023 09:05:37 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 377436B047E; Fri, 15 Dec 2023 09:05:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 224E06B047B for ; Fri, 15 Dec 2023 09:05:37 -0500 (EST) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E632EC0DBC for ; Fri, 15 Dec 2023 14:05:36 +0000 (UTC) X-FDA: 81569225472.09.1BE80FB Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf22.hostedemail.com (Postfix) with ESMTP id 057F9C002F for ; Fri, 15 Dec 2023 14:05:33 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702649134; 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=vngWSuQ5dtj8lCHO9ePl8zCR/BQ++F7es32yRt5NTA8=; b=w55dPyhx7giiVvt2AqB9nskeNrofZEBJro4mMYM0EoYKDZ9E7sTAGyradqm25cmRvAFb7p EMYnEbgV6cnMR359fnDWfDg1NXlB3oHtxSnH2JwILN/RiWo5FfSO1eT+eOVh4pW+J5P5dJ XcNeJZ735HoFulDZlkt6UgC8xXhdqsc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702649134; a=rsa-sha256; cv=none; b=g3KJvFu+LyGvpd6AYZSs8bOFmENLaNKox5PlcLghQFFkyh0Jv5Zo4FkoaXicFIG1UbzGPG BqXrJi7bEy+9VTD9xnwlrWQgX/ieBFDRJ8804TeCYj7OeW0C0m6GehbESwJh6Q4P4aq69K 2xdqLS1YI69/9v6xSqIUZG6lr9vdwtM= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=none; spf=pass (imf22.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com 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 25C08C15; Fri, 15 Dec 2023 06:06:18 -0800 (PST) Received: from [10.57.76.37] (unknown [10.57.76.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 68DE03F5A1; Fri, 15 Dec 2023 06:05:29 -0800 (PST) Message-ID: Date: Fri, 15 Dec 2023 14:05:28 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 14/15] arm64/mm: Implement ptep_set_wrprotects() to optimize fork() Content-Language: en-GB To: Alistair Popple Cc: Catalin Marinas , Will Deacon , Ard Biesheuvel , Marc Zyngier , Oliver Upton , James Morse , Suzuki K Poulose , Zenghui Yu , Andrey Ryabinin , Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Vincenzo Frascino , Andrew Morton , Anshuman Khandual , Matthew Wilcox , Yu Zhao , Mark Rutland , David Hildenbrand , Kefeng Wang , John Hubbard , Zi Yan , Barry Song <21cnbao@gmail.com>, Yang Shi , linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20231204105440.61448-1-ryan.roberts@arm.com> <20231204105440.61448-15-ryan.roberts@arm.com> <87cyvha2xd.fsf@nvdebian.thelocal> <87fs0413rx.fsf@nvdebian.thelocal> From: Ryan Roberts In-Reply-To: <87fs0413rx.fsf@nvdebian.thelocal> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Stat-Signature: uc44z9h4znnrsrbzy1ha5fraku89zzyb X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 057F9C002F X-Rspam-User: X-HE-Tag: 1702649133-56986 X-HE-Meta: U2FsdGVkX1/n0wiGFQ0maQ5qHii+xnEBGFlK2aRGO/M7M+IQWTEbJD9GoiMNyQCB912V1CKFqkSYsEFfXdMRwmfoI981kOUEoeccx+jn4dVUSJ0yj5iRb3VKzUfI65CCkdOrypRTOpL2rYKwddXQH5vjw0kR1um19e/ocQfaStlj6Sd/ErzL3nHI4VQk+Vw1o6qQxE/jb2tDMW03F1vEfbooczH2FSfEaIM8odF+7ciF+CP9+0WfIR2H35OhO0AMDUqFNqH62Hs9HYqLngOvyhQdYAuqP5qCiZcHnWBgQdixI4emXUmlFfN7EE0/OyS2QHgaOjFqz9qxdzY8LTUPRjUacwN4skJIx5LUp8lTLtJTU1SkxiIiA/MoubDiUhiGwZJNGt1Ha8KHk8s04xQPNjse7NW/WJRp29/lVv1N7XMooRZNrFDLlMxNqnnEIpufzH0uLE4DniJAtpkY2dlwpO9o4b0KWaunOQmvZdYes2teYdr3gwEuk5y/qr3vJf6mgAKRjy7AqKFPMWD3NWClyKSze9XjRQq93MZtqkxRf4gQOUMvDj6WQyMLv3lEjB6jXfDZIuO7VhVtf9Ekq1u9qjN0aS7PAmo1iHKkbVe6BLXWYnDVp+9JWJNSqRTzQroaFTCTfcGsfRg+r96T4foGYIrWUV9HdFmunesCQSgmtbaJwKt2DYmwU8y0sNuER32iA9rLzmdfmhLzl2DiTO46YgnU2teKtN4zsi79OpEiIfhw4lUot/8BLpgCAQKDbcAeU+6AlHtpj2NPlbsHmd8+QhWfU/rAjAmCdJDGSLysBaAuHmg9VerlZwW7yb1BCyTRQMh+f/FT56JdJWkTtg4qPbS1NhRk9hVOP4hHV5FLzcb6Jq00fFCZu7MY38UOdwtd4o4n+Nt7pY3U6laO7kotjTRnHORCXkyBhmh5aIVWJTRNu4qEA7BqYMuZgFUBHigQXGLPl53d65YQZf8P/kf 3TG6UoM2 hHH1AdxXx618rJGcrYVW4rGGabAWmRyv2jMfpg2DbJzWGTGk2HLaxYsKCZ9ksZFk1+c4ikXBtolBGtFGFh6QP2m3hWJikBNHkjFa2WOA2a7CXuaxtG8xX4cc9kCslmkzwdvT//oN/nCAK4dd4seiyxaOg1oRNIigAcsH5I7TfZ2x9cLEIJmUPT7k0nKDCVjQju8PNP2GB0fJHoHz4bqhNdi/ou2AfSiIsej4hiQyff6P9MnC23+uakT/Ak0JN2w45QO3S6UdnCNo48jf81EE62OoE3eWKQG/agWzYpzJWNIF9hllxlaiWy2Z1xVl8K0wBWCd6ZEHw4x9GhBQ= 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: List-Subscribe: List-Unsubscribe: On 15/12/2023 04:32, Alistair Popple wrote: > > Ryan Roberts writes: > >> On 08/12/2023 01:37, Alistair Popple wrote: >>> >>> Ryan Roberts writes: >>> >>>> With the core-mm changes in place to batch-copy ptes during fork, we can >>>> take advantage of this in arm64 to greatly reduce the number of tlbis we >>>> have to issue, and recover the lost fork performance incured when adding >>>> support for transparent contiguous ptes. >>>> >>>> If we are write-protecting a whole contig range, we can apply the >>>> write-protection to the whole range and know that it won't change >>>> whether the range should have the contiguous bit set or not. For ranges >>>> smaller than the contig range, we will still have to unfold, apply the >>>> write-protection, then fold if the change now means the range is >>>> foldable. >>>> >>>> This optimization is possible thanks to the tightening of the Arm ARM in >>>> respect to the definition and behaviour when 'Misprogramming the >>>> Contiguous bit'. See section D21194 at >>>> https://developer.arm.com/documentation/102105/latest/ >>>> >>>> Performance tested with the following test written for the will-it-scale >>>> framework: >>>> >>>> ------- >>>> >>>> char *testcase_description = "fork and exit"; >>>> >>>> void testcase(unsigned long long *iterations, unsigned long nr) >>>> { >>>> int pid; >>>> char *mem; >>>> >>>> mem = malloc(SZ_128M); >>>> assert(mem); >>>> memset(mem, 1, SZ_128M); >>>> >>>> while (1) { >>>> pid = fork(); >>>> assert(pid >= 0); >>>> >>>> if (!pid) >>>> exit(0); >>>> >>>> waitpid(pid, NULL, 0); >>>> >>>> (*iterations)++; >>>> } >>>> } >>>> >>>> ------- >>>> >>>> I see huge performance regression when PTE_CONT support was added, then >>>> the regression is mostly fixed with the addition of this change. The >>>> following shows regression relative to before PTE_CONT was enabled >>>> (bigger negative value is bigger regression): >>>> >>>> | cpus | before opt | after opt | >>>> |-------:|-------------:|------------:| >>>> | 1 | -10.4% | -5.2% | >>>> | 8 | -15.4% | -3.5% | >>>> | 16 | -38.7% | -3.7% | >>>> | 24 | -57.0% | -4.4% | >>>> | 32 | -65.8% | -5.4% | >>>> >>>> Signed-off-by: Ryan Roberts >>>> --- >>>> arch/arm64/include/asm/pgtable.h | 30 ++++++++++++++++++++--- >>>> arch/arm64/mm/contpte.c | 42 ++++++++++++++++++++++++++++++++ >>>> 2 files changed, 69 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >>>> index 15bc9cf1eef4..9bd2f57a9e11 100644 >>>> --- a/arch/arm64/include/asm/pgtable.h >>>> +++ b/arch/arm64/include/asm/pgtable.h >>>> @@ -984,6 +984,16 @@ static inline void __ptep_set_wrprotect(struct mm_struct *mm, >>>> } while (pte_val(pte) != pte_val(old_pte)); >>>> } >>>> >>>> +static inline void __ptep_set_wrprotects(struct mm_struct *mm, >>>> + unsigned long address, pte_t *ptep, >>>> + unsigned int nr) >>>> +{ >>>> + unsigned int i; >>>> + >>>> + for (i = 0; i < nr; i++, address += PAGE_SIZE, ptep++) >>>> + __ptep_set_wrprotect(mm, address, ptep); >>>> +} >>>> + >>>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE >>>> #define __HAVE_ARCH_PMDP_SET_WRPROTECT >>>> static inline void pmdp_set_wrprotect(struct mm_struct *mm, >>>> @@ -1139,6 +1149,8 @@ extern int contpte_ptep_test_and_clear_young(struct vm_area_struct *vma, >>>> unsigned long addr, pte_t *ptep); >>>> extern int contpte_ptep_clear_flush_young(struct vm_area_struct *vma, >>>> unsigned long addr, pte_t *ptep); >>>> +extern void contpte_set_wrprotects(struct mm_struct *mm, unsigned long addr, >>>> + pte_t *ptep, unsigned int nr); >>>> extern int contpte_ptep_set_access_flags(struct vm_area_struct *vma, >>>> unsigned long addr, pte_t *ptep, >>>> pte_t entry, int dirty); >>>> @@ -1290,13 +1302,25 @@ static inline int ptep_clear_flush_young(struct vm_area_struct *vma, >>>> return contpte_ptep_clear_flush_young(vma, addr, ptep); >>>> } >>>> >>>> +#define ptep_set_wrprotects ptep_set_wrprotects >>>> +static inline void ptep_set_wrprotects(struct mm_struct *mm, unsigned long addr, >>>> + pte_t *ptep, unsigned int nr) >>>> +{ >>>> + if (!contpte_is_enabled(mm)) >>>> + __ptep_set_wrprotects(mm, addr, ptep, nr); >>>> + else if (nr == 1) { >>> >>> Why do we need the special case here? Couldn't we just call >>> contpte_set_wrprotects() with nr == 1? >> >> My intention is for this to be a fast path for ptep_set_wrprotect(). I'm having >> to work hard to prevent regressing the order-0 folios case. > > This ends up calling three functions anyway so I'm curious - does > removing the one function call really make that much of difference? Yes; big time. All the functions in the fast path are inlined. The version regresses a fork() microbenchmark that David gave me by ~30%. I've had to work quite hard to reduce that to 2%, even from this starting point. There is so little in the inner loop that even the __ptep_get(ptep) (which is a READ_ONCE()) makes a measurable difference. Anyway, I'll be posting v4 with these optimizations and all the supporting benchmark data on Monday. > > Either way I think a comment justifying the special case (ie. that this > is simply a fast path for nr == 1) would be good. I've added a comment here in v4. > > Thanks. > >>> >>>> + contpte_try_unfold(mm, addr, ptep, __ptep_get(ptep)); >>>> + __ptep_set_wrprotects(mm, addr, ptep, 1); >>>> + contpte_try_fold(mm, addr, ptep, __ptep_get(ptep)); >>>> + } else >>>> + contpte_set_wrprotects(mm, addr, ptep, nr); >>>> +} >>>> + >>>> #define __HAVE_ARCH_PTEP_SET_WRPROTECT >>>> static inline void ptep_set_wrprotect(struct mm_struct *mm, >>>> unsigned long addr, pte_t *ptep) >>>> { >>>> - contpte_try_unfold(mm, addr, ptep, __ptep_get(ptep)); >>>> - __ptep_set_wrprotect(mm, addr, ptep); >>>> - contpte_try_fold(mm, addr, ptep, __ptep_get(ptep)); >>>> + ptep_set_wrprotects(mm, addr, ptep, 1); >>>> } >>>> >>>> #define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS >>>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c >>>> index e079ec61d7d1..2a57df16bf58 100644 >>>> --- a/arch/arm64/mm/contpte.c >>>> +++ b/arch/arm64/mm/contpte.c >>>> @@ -303,6 +303,48 @@ int contpte_ptep_clear_flush_young(struct vm_area_struct *vma, >>>> } >>>> EXPORT_SYMBOL(contpte_ptep_clear_flush_young); >>>> >>>> +void contpte_set_wrprotects(struct mm_struct *mm, unsigned long addr, >>>> + pte_t *ptep, unsigned int nr) >>>> +{ >>>> + unsigned long next; >>>> + unsigned long end = addr + (nr << PAGE_SHIFT); >>>> + >>>> + do { >>>> + next = pte_cont_addr_end(addr, end); >>>> + nr = (next - addr) >> PAGE_SHIFT; >>>> + >>>> + /* >>>> + * If wrprotecting an entire contig range, we can avoid >>>> + * unfolding. Just set wrprotect and wait for the later >>>> + * mmu_gather flush to invalidate the tlb. Until the flush, the >>>> + * page may or may not be wrprotected. After the flush, it is >>>> + * guarranteed wrprotected. If its a partial range though, we >>>> + * must unfold, because we can't have a case where CONT_PTE is >>>> + * set but wrprotect applies to a subset of the PTEs; this would >>>> + * cause it to continue to be unpredictable after the flush. >>>> + */ >>>> + if (nr != CONT_PTES) >>>> + contpte_try_unfold(mm, addr, ptep, __ptep_get(ptep)); >>>> + >>>> + __ptep_set_wrprotects(mm, addr, ptep, nr); >>>> + >>>> + addr = next; >>>> + ptep += nr; >>>> + >>>> + /* >>>> + * If applying to a partial contig range, the change could have >>>> + * made the range foldable. Use the last pte in the range we >>>> + * just set for comparison, since contpte_try_fold() only >>>> + * triggers when acting on the last pte in the contig range. >>>> + */ >>>> + if (nr != CONT_PTES) >>>> + contpte_try_fold(mm, addr - PAGE_SIZE, ptep - 1, >>>> + __ptep_get(ptep - 1)); >>>> + >>>> + } while (addr != end); >>>> +} >>>> +EXPORT_SYMBOL(contpte_set_wrprotects); >>>> + >>>> int contpte_ptep_set_access_flags(struct vm_area_struct *vma, >>>> unsigned long addr, pte_t *ptep, >>>> pte_t entry, int dirty) >>> >