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 0ECA4C021A4 for ; Thu, 13 Feb 2025 05:30:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2D9BC28000B; Thu, 13 Feb 2025 00:30:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2891F280007; Thu, 13 Feb 2025 00:30:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0DBC028000B; Thu, 13 Feb 2025 00:30:44 -0500 (EST) 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 DFFFE280007 for ; Thu, 13 Feb 2025 00:30:43 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 624C7C048A for ; Thu, 13 Feb 2025 05:30:43 +0000 (UTC) X-FDA: 83113796766.02.D0DA1B1 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf27.hostedemail.com (Postfix) with ESMTP id 456EE40008 for ; Thu, 13 Feb 2025 05:30:41 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf27.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1739424641; 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=GUchfY15ts6H155kQG7wVYsIBSGMOA4XTj4eVmmu35g=; b=k3UzbUpWmSzheMoNDQ3ZDaVo0m7KcQhj3ucOu5L2TwF6AhUeVuK2AijZN60unhrOIqlnqw +Fwx3LaX0szrbMbA0+EPX/oMMdT73Cj34BoXOYdEyTgBvKE4cwqpEkDZgLhx6aqcApgWDs lCvRh2m+tyel8h6t17hkLUuDz1El1O0= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf27.hostedemail.com: domain of anshuman.khandual@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=anshuman.khandual@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1739424641; a=rsa-sha256; cv=none; b=ibnP31Luh9JuU15HNjisOVmL8bmYf+OKDK1bvsVSL7FFqOgJqqAz73lSRWswq54UGVG0wH /ZqZNkXjMGgGNnbVTgyerTHEvJTDP+aihz3SuhNBxEpWZhXX0EjzbmGXOTrYwzSg+8ZxgU 6pDQEu8v6Pi3vXFqBIOLdIGjQ/LKvqo= 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 3A9E71756; Wed, 12 Feb 2025 21:31:01 -0800 (PST) Received: from [10.162.16.135] (a077893.blr.arm.com [10.162.16.135]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C00063F6A8; Wed, 12 Feb 2025 21:30:35 -0800 (PST) Message-ID: <0052097e-2284-4f9e-b37c-2ca2de527667@arm.com> Date: Thu, 13 Feb 2025 11:00:32 +0530 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 16/16] arm64/mm: Defer barriers when updating kernel mappings To: Ryan Roberts , Catalin Marinas , Will Deacon , Muchun Song , Pasha Tatashin , Andrew Morton , Uladzislau Rezki , Christoph Hellwig , Mark Rutland , Ard Biesheuvel , Dev Jain , Alexandre Ghiti , Steve Capper , Kevin Brodsky Cc: linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20250205151003.88959-1-ryan.roberts@arm.com> <20250205151003.88959-17-ryan.roberts@arm.com> <9bc5527e-16f4-45cc-aced-55b1ace6c143@arm.com> Content-Language: en-US From: Anshuman Khandual In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 456EE40008 X-Stat-Signature: oynqmh84dpme9hfugbohd61jqshf4sbn X-HE-Tag: 1739424641-535338 X-HE-Meta: U2FsdGVkX18qGc7heac0sIbzczrVHZIe/eXXT3Zjz84Q5pcEasIulSF8Qw4/e6/qyDcCNshc/30mInGUWpstnDjiewbm98amv+qtkFSBOYiet7q/MLh8bsQR62rJKwsQg6bZfUOwBl7OjGZqg3XYIF/DuhbZih3x2Xu2a4PAqjFh1D7UXwNP1k+FhBgZFzJDqc2XVIUjCCb/5cBvbQbk/9othKLeb5YD1nEm8tIu4Y+EsOxw4JQ7dqKJuWaiZEM+Vyz07whPLm3WovUuItb7P+siAEEbEnE42SDYZwlVn2SzGiCZcNmhuChiarHEMBoOFCNX53he+lMRIIOAdBTTS2TPg8LwNuWOM6ovGY2fQyKOv7Eye+MgzVylacHeL0QSE+bslfyutc252NVy0KW9lVyyM3tGyxYh69WTFznbZzM3smv5lyQruE1VY4HsmW4NqeZYrMnb2JZQ7SOqKJQWfiI1jI6yoJrLk4oM4vrnr3pJ2xaqyjWawdP4Fq2BiPgVz7sDvtdi0R9BTB94Zhw+Fl+R1LgD5/G9M5sU7pkESO2NMTzbE+6l+wjVi3CloUShT4BB8J8KGwPck8VB8m/6k2TCsryDv7dLk66llP8yO/Jssn1aqLXFUv2h5kIlO+JD3LMBLD3P4eQF0rqLxRy0mCJZuU8+QWa23YSr6SubxWysSCnkeqf0wHlyWZggw4rFjZAp6r4RQviqrmY3u/YPWcBpMZH0UB39J4Nua8D3eN/szIZwZ7iTXGpp18PxHzuN+hwAdHwKz06oE4Nup9cyvx8UryfOorSLMag/nhVbboapV4TurKwbW/m/HuMMT5Xm3oaIfzeZo1oqfxvfIdTWCowK2j0GEQJmTpphflcj0vykBUistq6gkfSBsVJBcPgF4ZPoSFkviT0nYO4m27TenGAPI0KkDz0NFi7YmJoe8ujP8bjOBzYtE84dAceM6fBZArcZZ/MH9dVv+T5HTG9 qFH8+mpo 1VAiHf4RrlnklBK4UlPACJviVFwy6LrxBVyajV2TzeDIYt8moDAyOnXePicNf35Ee2L6Wgrfi/qbgzHWu0kZHcJplhXrdw/xhsckw6HBf2tyPke9BWRBBWYEXmbKDQ58/mO8PG9XKUhYx/SrYbXHjQgbfyzZtRYTq98rwc/fQg7UjyoBmUH+aipyIBVVbadI0OVwhATkOHYE/sag= 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 2/10/25 16:42, Ryan Roberts wrote: > On 10/02/2025 08:03, Anshuman Khandual wrote: >> >> >> On 2/5/25 20:39, Ryan Roberts wrote: >>> Because the kernel can't tolerate page faults for kernel mappings, when >>> setting a valid, kernel space pte (or pmd/pud/p4d/pgd), it emits a >>> dsb(ishst) to ensure that the store to the pgtable is observed by the >>> table walker immediately. Additionally it emits an isb() to ensure that >>> any already speculatively determined invalid mapping fault gets >>> canceled.> >>> We can improve the performance of vmalloc operations by batching these >>> barriers until the end of a set up entry updates. The newly added >>> arch_update_kernel_mappings_begin() / arch_update_kernel_mappings_end() >>> provide the required hooks. >>> >>> vmalloc improves by up to 30% as a result. >>> >>> Two new TIF_ flags are created; TIF_KMAP_UPDATE_ACTIVE tells us if we >>> are in the batch mode and can therefore defer any barriers until the end >>> of the batch. TIF_KMAP_UPDATE_PENDING tells us if barriers are queued to >>> be emited at the end of the batch. >> >> Why cannot this be achieved with a single TIF_KMAP_UPDATE_ACTIVE which is >> set in __begin(), cleared in __end() and saved across a __switch_to(). > > So unconditionally emit the barriers in _end(), and emit them in __switch_to() > if TIF_KMAP_UPDATE_ACTIVE is set? Right. > > I guess if calling _begin() then you are definitely going to be setting at least > 1 PTE. So you can definitely emit the barriers unconditionally. I was trying to > protect against the case where you get pre-empted (potentially multiple times) > while in the loop. The TIF_KMAP_UPDATE_PENDING flag ensures you only emit the > barriers when you definitely need to. Without it, you would have to emit on > every pre-emption even if no more PTEs got set. > > But I suspect this is a premature optimization. Probably it will never occur. So Agreed. > I'll simplify as you suggest. > > Thanks, > Ryan > >> >>> >>> Signed-off-by: Ryan Roberts >>> --- >>> arch/arm64/include/asm/pgtable.h | 65 +++++++++++++++++++--------- >>> arch/arm64/include/asm/thread_info.h | 2 + >>> arch/arm64/kernel/process.c | 20 +++++++-- >>> 3 files changed, 63 insertions(+), 24 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >>> index ff358d983583..1ee9b9588502 100644 >>> --- a/arch/arm64/include/asm/pgtable.h >>> +++ b/arch/arm64/include/asm/pgtable.h >>> @@ -39,6 +39,41 @@ >>> #include >>> #include >>> #include >>> +#include >>> + >>> +static inline void emit_pte_barriers(void) >>> +{ >>> + dsb(ishst); >>> + isb(); >>> +} >> >> There are many sequence of these two barriers in this particular header, >> hence probably a good idea to factor this out into a common helper. >>>> + >>> +static inline void queue_pte_barriers(void) >>> +{ >>> + if (test_thread_flag(TIF_KMAP_UPDATE_ACTIVE)) { >>> + if (!test_thread_flag(TIF_KMAP_UPDATE_PENDING)) >>> + set_thread_flag(TIF_KMAP_UPDATE_PENDING); >>> + } else >>> + emit_pte_barriers(); >>> +} >>> + >>> +#define arch_update_kernel_mappings_begin arch_update_kernel_mappings_begin >>> +static inline void arch_update_kernel_mappings_begin(unsigned long start, >>> + unsigned long end) >>> +{ >>> + set_thread_flag(TIF_KMAP_UPDATE_ACTIVE); >>> +} >>> + >>> +#define arch_update_kernel_mappings_end arch_update_kernel_mappings_end >>> +static inline void arch_update_kernel_mappings_end(unsigned long start, >>> + unsigned long end, >>> + pgtbl_mod_mask mask) >>> +{ >>> + if (test_thread_flag(TIF_KMAP_UPDATE_PENDING)) >>> + emit_pte_barriers(); >>> + >>> + clear_thread_flag(TIF_KMAP_UPDATE_PENDING); >>> + clear_thread_flag(TIF_KMAP_UPDATE_ACTIVE); >>> +} >>> >>> #ifdef CONFIG_TRANSPARENT_HUGEPAGE >>> #define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE >>> @@ -323,10 +358,8 @@ static inline void __set_pte_complete(pte_t pte) >>> * Only if the new pte is valid and kernel, otherwise TLB maintenance >>> * or update_mmu_cache() have the necessary barriers. >>> */ >>> - if (pte_valid_not_user(pte)) { >>> - dsb(ishst); >>> - isb(); >>> - } >>> + if (pte_valid_not_user(pte)) >>> + queue_pte_barriers(); >>> } >>> >>> static inline void __set_pte(pte_t *ptep, pte_t pte) >>> @@ -791,10 +824,8 @@ static inline void set_pmd(pmd_t *pmdp, pmd_t pmd) >>> >>> WRITE_ONCE(*pmdp, pmd); >>> >>> - if (pmd_valid_not_user(pmd)) { >>> - dsb(ishst); >>> - isb(); >>> - } >>> + if (pmd_valid_not_user(pmd)) >>> + queue_pte_barriers(); >>> } >>> >>> static inline void pmd_clear(pmd_t *pmdp) >>> @@ -869,10 +900,8 @@ static inline void set_pud(pud_t *pudp, pud_t pud) >>> >>> WRITE_ONCE(*pudp, pud); >>> >>> - if (pud_valid_not_user(pud)) { >>> - dsb(ishst); >>> - isb(); >>> - } >>> + if (pud_valid_not_user(pud)) >>> + queue_pte_barriers(); >>> } >>> >>> static inline void pud_clear(pud_t *pudp) >>> @@ -960,10 +989,8 @@ static inline void set_p4d(p4d_t *p4dp, p4d_t p4d) >>> >>> WRITE_ONCE(*p4dp, p4d); >>> >>> - if (p4d_valid_not_user(p4d)) { >>> - dsb(ishst); >>> - isb(); >>> - } >>> + if (p4d_valid_not_user(p4d)) >>> + queue_pte_barriers(); >>> } >>> >>> static inline void p4d_clear(p4d_t *p4dp) >>> @@ -1098,10 +1125,8 @@ static inline void set_pgd(pgd_t *pgdp, pgd_t pgd) >>> >>> WRITE_ONCE(*pgdp, pgd); >>> >>> - if (pgd_valid_not_user(pgd)) { >>> - dsb(ishst); >>> - isb(); >>> - } >>> + if (pgd_valid_not_user(pgd)) >>> + queue_pte_barriers(); >>> } >>> >>> static inline void pgd_clear(pgd_t *pgdp) >>> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h >>> index 1114c1c3300a..382d2121261e 100644 >>> --- a/arch/arm64/include/asm/thread_info.h >>> +++ b/arch/arm64/include/asm/thread_info.h >>> @@ -82,6 +82,8 @@ void arch_setup_new_exec(void); >>> #define TIF_SME_VL_INHERIT 28 /* Inherit SME vl_onexec across exec */ >>> #define TIF_KERNEL_FPSTATE 29 /* Task is in a kernel mode FPSIMD section */ >>> #define TIF_TSC_SIGSEGV 30 /* SIGSEGV on counter-timer access */ >>> +#define TIF_KMAP_UPDATE_ACTIVE 31 /* kernel map update in progress */ >>> +#define TIF_KMAP_UPDATE_PENDING 32 /* kernel map updated with deferred barriers */ >>> >>> #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) >>> #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) >>> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c >>> index 42faebb7b712..1367ec6407d1 100644 >>> --- a/arch/arm64/kernel/process.c >>> +++ b/arch/arm64/kernel/process.c >>> @@ -680,10 +680,10 @@ struct task_struct *__switch_to(struct task_struct *prev, >>> gcs_thread_switch(next); >>> >>> /* >>> - * Complete any pending TLB or cache maintenance on this CPU in case >>> - * the thread migrates to a different CPU. >>> - * This full barrier is also required by the membarrier system >>> - * call. >>> + * Complete any pending TLB or cache maintenance on this CPU in case the >>> + * thread migrates to a different CPU. This full barrier is also >>> + * required by the membarrier system call. Additionally it is required >>> + * for TIF_KMAP_UPDATE_PENDING, see below. >>> */ >>> dsb(ish); >>> >>> @@ -696,6 +696,18 @@ struct task_struct *__switch_to(struct task_struct *prev, >>> /* avoid expensive SCTLR_EL1 accesses if no change */ >>> if (prev->thread.sctlr_user != next->thread.sctlr_user) >>> update_sctlr_el1(next->thread.sctlr_user); >>> + else if (unlikely(test_thread_flag(TIF_KMAP_UPDATE_PENDING))) { >>> + /* >>> + * In unlikely event that a kernel map update is on-going when >>> + * preemption occurs, we must emit_pte_barriers() if pending. >>> + * emit_pte_barriers() consists of "dsb(ishst); isb();". The dsb >>> + * is already handled above. The isb() is handled if >>> + * update_sctlr_el1() was called. So only need to emit isb() >>> + * here if it wasn't called. >>> + */ >>> + isb(); >>> + clear_thread_flag(TIF_KMAP_UPDATE_PENDING); >>> + } >>> >>> /* the actual thread switch */ >>> last = cpu_switch_to(prev, next); >