From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A3B3D1F8919 for ; Wed, 22 Jan 2025 10:34:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737542100; cv=none; b=acsoczAwodoI6KbTQcTEH/dsjpLW7C+PDG/OLMnGwoAhM71UNRuHVBtWb4jh1xdz/hvE8w49lBhkmyKtDnSVVMhPCTy+3YkmMjfBh44Jf9H1UyDrWirDvatWk7p7iWcxMBmoDysDXWKOJWF2uAZdILG/xRpoXfsw5AI8UxoRNog= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737542100; c=relaxed/simple; bh=ccJd2MyaJcrtIWCpembBa49egq7fiecO/LfSNbaFH/g=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References: MIME-Version:Content-Type; b=nqRujTtzKvgB5U6MrFbx570sy0zXTeWRENdltzvghTzVoDJqnY6b2MhLC/AwWf10uOb5TRFULwFShyReqUhjqbjGZaWb09x0X+BrnfNH9zqVQdhrvTOQc6e+qiEAR6t3pUIwrmMK7qW7FeSJ9xq0wvliATLgabEaT3pB3vTRQeE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=Ak5IHfNw; arc=none smtp.client-ip=209.85.216.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="Ak5IHfNw" Received: by mail-pj1-f48.google.com with SMTP id 98e67ed59e1d1-2f43da61ba9so8819015a91.2 for ; Wed, 22 Jan 2025 02:34:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1737542098; x=1738146898; darn=vger.kernel.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=8U/payZ20c9Ax3t2jNC8bRsYJo5GmzmF42je0IW1q6s=; b=Ak5IHfNwtgfzutUxLesTdfAY0lOXs/3lRRGZpeSlI7gwqP5/JhPaJbErOrxJfDEbrY D63oKO13eHh2EqG/7d5xAgcBIOtY9gjRGHuvO8/xejIdi5B2sHTJfoKnPqy4xwEv5jvo IYQ2NICajTgz5+gn3xiOicGm2t8+YXlJhcxs3nwcGgpc0Nw6rQpJptWmHEru8UjMYDvc J88lYTYWjpr2zt1EOYmHcafya4V5zrzjqBa502desY3O5vbE4myukbc7Ns+oQoScSf/i ACi9cX6kpOEP7cNUYWDuM3aaeBcOXfWj/SVEDkVmnnQ3NaqM0k8ySbqFMmTQM7OBnorW 1p0Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737542098; x=1738146898; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=8U/payZ20c9Ax3t2jNC8bRsYJo5GmzmF42je0IW1q6s=; b=SmMoogAeWBK/LULpfdUXDFxckEmgjc6Us19xNqcUx71ezTHivjJ7WUS6X6yZ4n3ydp RzdUEg+G/v/xBnGVxpBWcEm97U+Nngm/8Bij0v8r2WvClt8pspi0d3zAggHUjKByDVO0 KKNt7611dXPnFwvHbKReqnvgR+emVMKwlNu9aWT4gQ5g7Gbc6DIj/7FJwba3eBh58VBt cRSuBq0Allcu4/yKT+hrf5dCi9Bf+bHZwng8YZNhpBJVdLGDdjbvJDOseLwxmIREKsU7 33A1wp8AvtOXss2JpjgmzkvqXcmoBXgvR+z17hNHsuoW4aKE1ByzCtJzvE8qh5tWRcnJ JMdA== X-Gm-Message-State: AOJu0Yzz6VTfJ0fnjG5/xa/ecMBR8D7931xYeqm2tJ2E+ksW33yH3UMT 2ddQqzYihvCCclh7o26DppmWrscsr2eALRXqgvWDx9hecIfvztDsiOFoP0Awzg== X-Gm-Gg: ASbGncvX+QPZ1bcnPux4aZSP79UZeMBUaXvfgL/gUZ/g7CuvsDaxBprtB8DnqdK4tBg SetPPwWOxd4VtHTdZjlX03IthSJnASCjxJYoWsufvTUACnfEpBh9l+77g7kfBEeqE3DDAJlDn55 FfY6p+jkMi8FbeajIk0skdtqGrzj9GRNqA9JXw5vnZrMO6Q5T+hM+SJy3EAGlp1/Jez3esMDLH5 sb/aj3lbsjBeSyXSuLAEFz24jUQcHMeOYNTz3ij5Lg3hpfM6wwBayoEaVByL4rA9RavRg11mLdG lXKINoyleoyJHsknQrKBlrF3QTTerkjAWa+3qswqZjYayfU3EVnY4g== X-Google-Smtp-Source: AGHT+IEA7uPz8eU9TDBP3U4S5r/5kxWYft7JncmGKt91LbEvuF4KcW1eWWFG5Z0CDvmZRNuxUgbBtg== X-Received: by 2002:a05:6a00:428d:b0:725:ae5f:7f06 with SMTP id d2e1a72fcca58-72dafadbc37mr29647191b3a.23.1737542097673; Wed, 22 Jan 2025 02:34:57 -0800 (PST) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72dabaa751asm10651865b3a.162.2025.01.22.02.34.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Jan 2025 02:34:56 -0800 (PST) Date: Wed, 22 Jan 2025 02:34:45 -0800 (PST) From: Hugh Dickins To: Roman Gushchin cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Jann Horn , Peter Zijlstra , Will Deacon , "Aneesh Kumar K.V" , Nick Piggin , Hugh Dickins , linux-arch@vger.kernel.org Subject: Re: [PATCH] mmu_gather: move tlb flush for VM_PFNMAP/VM_MIXEDMAP vmas into free_pgtables() In-Reply-To: <20250121200929.188542-1-roman.gushchin@linux.dev> Message-ID: References: <20250121200929.188542-1-roman.gushchin@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Tue, 21 Jan 2025, Roman Gushchin wrote: > Commit b67fbebd4cf9 ("mmu_gather: Force tlb-flush VM_PFNMAP vmas") > added a forced tlbflush to tlb_vma_end(), Yes, I think that was a poor way of fixing the bug in question. > 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). Right. > > 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()"). Something like this patch will be a good improvement: but not this version of the patch. Because the mmu_gather may be gathering across many vmas, but tlb_start_vma(), well, its "tlb_update_vma_flags()", says tlb->vma_pfn = !!(vma->vm_flags & (VM_PFNMAP|VM_MIXEDMAP)); so a following vma may reset vma_pfn too soon: more care is needed. But probably vma_pfn should be reset to 0 somewhere, to avoid an extra TLB flush in free_pgtables() when it has already been done. Perhaps vma_pfn should follow the same pattern of initialization, setting and clearing as cleared_ptes etc, instead of following vma_huge and vma_exec. Perhaps, but it is something different, and I've not yet checked enough to be sure: tlb.h is still a maze too twisty for me. Hugh (after power outage interrupted reply) > > Note, that if tlb->fullmm is set, no flush is required, as the whole > mm is about to be destroyed. > > 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 | 16 ++++------------ > mm/memory.c | 7 +++++++ > 2 files changed, 11 insertions(+), 12 deletions(-) > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > index 709830274b75..411daa96f57a 100644 > --- a/include/asm-generic/tlb.h > +++ b/include/asm-generic/tlb.h > @@ -549,22 +549,14 @@ static inline void tlb_start_vma(struct mmu_gather *tlb, struct vm_area_struct * > > static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vma) > { > - if (tlb->fullmm) > + if (tlb->fullmm || IS_ENABLED(CONFIG_MMU_GATHER_MERGE_VMAS)) > return; > > /* > - * VM_PFNMAP is more fragile because the core mm will not track the > - * page mapcount -- there might not be page-frames for these PFNs after > - * all. Force flush TLBs for such ranges to avoid munmap() vs > - * unmap_mapping_range() races. > + * Do a TLB flush and reset the range at VMA boundaries; this avoids > + * the ranges growing with the unused space between consecutive VMAs. > */ > - if (tlb->vma_pfn || !IS_ENABLED(CONFIG_MMU_GATHER_MERGE_VMAS)) { > - /* > - * Do a TLB flush and reset the range at VMA boundaries; this avoids > - * the ranges growing with the unused space between consecutive VMAs. > - */ > - tlb_flush_mmu_tlbonly(tlb); > - } > + tlb_flush_mmu_tlbonly(tlb); > } > > /* > diff --git a/mm/memory.c b/mm/memory.c > index 398c031be9ba..2071415f68dd 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -365,6 +365,13 @@ void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas, > { > struct unlink_vma_file_batch vb; > > + /* > + * Ensure we have no stale TLB entries by the time this mapping is > + * removed from the rmap. > + */ > + if (tlb->vma_pfn && !tlb->fullmm) > + tlb_flush_mmu(tlb); > + > do { > unsigned long addr = vma->vm_start; > struct vm_area_struct *next; > -- > 2.48.0.rc2.279.g1de40edade-goog