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 95523C02182 for ; Thu, 23 Jan 2025 16:45:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2B0D7280001; Thu, 23 Jan 2025 11:45:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 260EB6B009B; Thu, 23 Jan 2025 11:45:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1289B280001; Thu, 23 Jan 2025 11:45:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id E63F26B009A for ; Thu, 23 Jan 2025 11:45:52 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 8F93F12012B for ; Thu, 23 Jan 2025 16:45:52 +0000 (UTC) X-FDA: 83039293344.24.ED3541D Received: from out-180.mta1.migadu.com (out-180.mta1.migadu.com [95.215.58.180]) by imf30.hostedemail.com (Postfix) with ESMTP id 604FF80016 for ; Thu, 23 Jan 2025 16:45:50 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=kJ8nr8AX; spf=pass (imf30.hostedemail.com: domain of roman.gushchin@linux.dev designates 95.215.58.180 as permitted sender) smtp.mailfrom=roman.gushchin@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1737650750; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=cJ9ZmMqOR4SDwDKQYZ4A34XUruIirQBnXbcnNkA+BHQ=; b=hK7eCmOJdW9l23pOJ9kG02V38FjV0o/ufTlcknqBsqDwdcNkWpH4oAQA1dKY4wih7qy6BT PNaF2uRtWuJ6JnoXHb/rYjRLZRmeJphFVkqbadhlTGoR5+NEUV1T+5uXn3OjMViWLXgA1/ q7Y5B7hJcmFWW1y/1nTltOfpb7LI7Fc= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1737650750; a=rsa-sha256; cv=none; b=rWbLZ3m0M45z8fJcFGPJg5OGfkRCOp0k60CUo1RXQQEbth/JY2bMAifurvXB1iV8BkcqF4 KKEOUYyOcd9uCMN8V8cZK1GpmHDAvsTllxSAEGN/JW4xftmKkcW3MZ4hGlOfzXASgl4rui vBjPN99j60c3AHSd1gF3GE5HQP0u7KU= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=kJ8nr8AX; spf=pass (imf30.hostedemail.com: domain of roman.gushchin@linux.dev designates 95.215.58.180 as permitted sender) smtp.mailfrom=roman.gushchin@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Date: Thu, 23 Jan 2025 16:45:42 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1737650748; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=cJ9ZmMqOR4SDwDKQYZ4A34XUruIirQBnXbcnNkA+BHQ=; b=kJ8nr8AXkvkQKmH/qZSX2NTzacDgublgbX56q14JVCxISLaRgcjD1rGphWFyTcdGYrGI0X b6IRJ9OxDGdbm2nAX4y2CpjgrizqJsUt8HRvf05MioMF4x85aJNFNsnhsIavSUY3qP+LBp qHuyk0HiiKLFZZFFfLxfjAljgtnfNrc= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Roman Gushchin To: Hugh Dickins Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Jann Horn , Peter Zijlstra , Will Deacon , "Aneesh Kumar K.V" , Nick Piggin , linux-arch@vger.kernel.org Subject: Re: [PATCH v2] mmu_gather: move tlb flush for VM_PFNMAP/VM_MIXEDMAP vmas into free_pgtables() Message-ID: References: <20250122232716.1321171-1-roman.gushchin@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-Rspamd-Queue-Id: 604FF80016 X-Stat-Signature: wwx6quyhtd1ubapfb41kehbo1yscnpfp X-Rspam-User: X-Rspamd-Server: rspam12 X-HE-Tag: 1737650750-906370 X-HE-Meta: U2FsdGVkX1+xMcBI60JXCiv6ROVoeUsw51Eb8w2BTuXrqeJhCm6Y6Huv8wZX7n5Ot325+pHLBxoo1DhjdoYPSVqAd+EW7F/jLEOO345DfmNmIaJpQVvyD70W0sn8/MmWiUCDV4HJRYpDFknLfiTuqVP4eqFqnMXswZgZFvG5TgymvyEHuSrAicXNF2sC9dRn6r9b5ab1ATcSsbVSnq4stCULk84gs/HEmGEsUrkOFqmGjuFdPK5JVQCBHrYwhsUTQ9tNuRW/ptjJ57L9ZRup4jKg3MNL8O4rVBqbF52TcAbhPbIYWGbEW2F/wdNr6kOtZ838Td7vxDrY9p6n19LxHkgnlg6aoCiAskevsfLCyaJy0iCjjFQT17w8O+aikmRStmnZ4oRYfL6jV/Sppr+09ChAaapKQRTx3RVvNERQ8rqr7qx+16zrQEis9xcRvLaIJT7HJKx4f8HyWcHe+T11J3OCQYFuY0ouMUdAME45O8MbK/AK+K6GW2DH0L8FZhC8uQsRoi1JwOrdWm8MKCUMBJfBLbBqEmdplRdoARIVdWC9VKVeAysswjY5clgtnjIo4hD/3TJTNkfZQBKP+i/vzaTkA7pnr7vG/gxAPLtEeYtPN1cUU/rAojsDynsqV5OzyJt2d6eI4KJXcvTZlpfZwOR8HYyvLQ0+rQGRh94axOJ1RqWmvrPoGLalNvewnsESgaZ9mj+Aqs0yilQ/6WjnVtMgt4zO+2b5yEYDz9Ym9DKZOnn7oO/Ca7nF/AWb8Zkj9jThn0hK7mjnSadr2u5TonvQDv1LL0bZWiiIghAZ4pOecI+yjp9P6n10EA20MZFcU8Os4cQ+JqqVMXrsUPQf4pJRV5lOL2Yw7BSpukZUvwdykmdSHvxMwFZPbsXJoSNwV9vnmp7F3bmPBagkJF8bdEST3E4aJiuiRJEqFY/N71SG/UE86J1wvuNngtaFDTtHkToQAd6HSCnboYQlZ5E CLCruaOf yzepPPVL3RIUU6nlu3VY8I9rcVMHMbGt8n+26P03efgX5O8CZTSHw2dr6uorBsfhyKPEHKiyiuLAS3SOuO9urKM0d91KIZaAyHlnK2hO3FLsJfKiz9CrbJfnV/RyF3XlutKplEOGNlKj47g1LgDKpbJlfbHmZ3Qfw174WT9RdRFvEaS2IuoBvUMKfryLKN1ewKvi4SDgfdrJCciDSlM0uFvxAo5ShyO6pl4j70axEsSuEu6b4+PZqpFGTyJNhgIcYphlP/3y2DqSybQH+gk0brNc06+wvC7YiI1MP0aIWEBEdDJdmu9dV0snfVOp1Vx0ORNvvSPD68y+/n9KLeoCib/F+uUXXeTukenhcbZwXk8s92r+/Bj+B3rbeytYtt/cTDH25ushGyWE53FPTHAsMfPMo53h7eSXVYUMmt652AGEnaYTC5U/sKLZN5jco5CEXiVyaElitC0kzCNSiV03gFlfsBo6Nt8GTVXsGIDbn+1H3f5fu+9iiv9kLZK4queyPVxG8 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 Wed, Jan 22, 2025 at 11:42:13PM -0800, Hugh Dickins wrote: > On Wed, 22 Jan 2025, Roman Gushchin wrote: > > > Commit b67fbebd4cf9 ("mmu_gather: Force tlb-flush VM_PFNMAP vmas") > > added a forced tlbflush to tlb_vma_end(), which is required to avoid a > > race between munmap() and unmap_mapping_range(). However it added some > > overhead to other paths where tlb_vma_end() is used, but vmas are not > > removed, e.g. madvise(MADV_DONTNEED). > > > > Fix this by moving the tlb flush out of tlb_end_vma() into > > free_pgtables(), somewhat similar to the stable version of the > > original commit: e.g. stable commit 895428ee124a ("mm: Force TLB flush > > for PFNMAP mappings before unlink_file_vma()"). > > > > Note, that if tlb->fullmm is set, no flush is required, as the whole > > mm is about to be destroyed. > > > > v2: > > - moved vma_pfn flag handling into tlb.h (by Peter Z.) > > - added comments (by Peter Z.) > > - fixed the vma_pfn flag setting (by Hugh D.) > > > > Suggested-by: Jann Horn > > Signed-off-by: Roman Gushchin > > Cc: Peter Zijlstra > > Cc: Will Deacon > > Cc: "Aneesh Kumar K.V" > > Cc: Andrew Morton > > Cc: Nick Piggin > > Cc: Hugh Dickins > > Cc: linux-arch@vger.kernel.org > > Cc: linux-mm@kvack.org > > --- > > include/asm-generic/tlb.h | 41 ++++++++++++++++++++++++++------------- > > mm/memory.c | 7 +++++++ > > 2 files changed, 35 insertions(+), 13 deletions(-) > > > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > > index 709830274b75..fbe31f49a5af 100644 > > --- a/include/asm-generic/tlb.h > > +++ b/include/asm-generic/tlb.h > > @@ -449,7 +449,14 @@ tlb_update_vma_flags(struct mmu_gather *tlb, struct vm_area_struct *vma) > > */ > > tlb->vma_huge = is_vm_hugetlb_page(vma); > > tlb->vma_exec = !!(vma->vm_flags & VM_EXEC); > > - tlb->vma_pfn = !!(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)); > > + > > + /* > > + * vma_pfn is checked and cleared by tlb_flush_mmu_pfnmap() > > + * for a set of vma's, so it should be set if at least one vma > > + * has VM_PFNMAP or VM_MIXEDMAP flags set. > > + */ > > + if (vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)) > > + tlb->vma_pfn = 1; > > Okay, but struct mmu_gather is usually on the caller's stack, > containing junk initially, and there's nothing in this patch > yet to initialize tlb->vma_pfn to 0. > > __tlb_reset_range() needs to do that, doesn't it? With some adjustment > to its comment about not resetting mmu_gather::vma_* fields. Or would > it be better to get around that by renaming vma_pfn to, er, something > else - I'd have to understand the essence of Jann's race better to > come up with the right name - untracked_mappings? would that be right? > I still haven't grasped the essence of that race. Yeah, this is a really good point. > > (Panic attack: where is, for example, tlb->need_flush_all initialized > to 0? Ah, over in mm/mmu_gather.c, __tlb_gather_mmu(). Phew.) > > And if __tlb_reset_range() resets tlb->vma_pfn to 0, then that has the > side-effect that any TLB flush cancels the vma_pfn state: which is a > desirable side-effect, isn't it? avoiding the possibility of doing an > unnecessary extra TLB flush in free_pgtables(), as I criticized before. Good point. Just sent out v3 with your suggestions. Thank you for your help here! Roman