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 X-Spam-Level: X-Spam-Status: No, score=-15.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EEECCC388F9 for ; Mon, 23 Nov 2020 21:18:35 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5AD23206D4 for ; Mon, 23 Nov 2020 21:18:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ZLxgyYlv"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="XY4IMJzj" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5AD23206D4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0rTXq3gEDnydl7aL2GeLevCnL0FtvVqzMDWTccDWQGA=; b=ZLxgyYlvrP2LQF+6rOQ/bbYzl mGkDCrmpf0uxGA+xRhiccC/quvFlpJtfEYxXWZL9WlBkNK3TaboKNyBRO9OHIDdzU+gqDb5pThtKE YwlqrlPfoS2YjCGZ+HJhbz8cQqwUcO7ock/Wq6qP+G5jN2UU+0mLQS2uPJdV0a8UstqFCEwmFYj9q 9Zq32TswDj+lZ30dJTzolDiP8UHEZLia1J/pWiFMknwt1D0GSFY8CrjSsEroEx/DVT1CKnrDT4As3 ZRy6DaoUEvfuu1QxRwPlRNWNPAoXGMhrxj77Szj71WNV5KnPWG8NjVi6CcRdtkAy5JNhQIArugLkE /mZFKYddg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1khJDl-0002oK-BY; Mon, 23 Nov 2020 21:18:01 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1khJDh-0002nK-UW for linux-arm-kernel@lists.infradead.org; Mon, 23 Nov 2020 21:17:59 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1C4BE206B5; Mon, 23 Nov 2020 21:17:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1606166276; bh=zbTKH20VVZucoaew5ZXWATj6gCGP2JnDyZG1DT3kGTI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=XY4IMJzjKhC2/RYHNdBmKxgJiiRAean7wQCa7lewSt3Xm56dKXrzNeJcNRGYORfpG YqWohL+HmE81nh3gpZWmE/tS9caSMs5+NeN2M6LFWR5uWAUH60m0S+dbgAPeckdjqG CGmFjwEV6GKCUIpGsgTkQzcW4hHbCAoSvVGmS5ms= Date: Mon, 23 Nov 2020 21:17:51 +0000 From: Will Deacon To: Yu Zhao Subject: Re: [PATCH 6/6] mm: proc: Avoid fullmm flush for young/dirty bit toggling Message-ID: <20201123211750.GA12069@willie-the-truck> References: <20201120143557.6715-1-will@kernel.org> <20201120143557.6715-7-will@kernel.org> <20201120204005.GC1303870@google.com> <20201123183554.GC11688@willie-the-truck> <20201123200403.GA3888699@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201123200403.GA3888699@google.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201123_161758_276293_0BA46BBD X-CRM114-Status: GOOD ( 36.24 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel-team@android.com, Anshuman Khandual , Peter Zijlstra , Catalin Marinas , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Minchan Kim , Linus Torvalds , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Nov 23, 2020 at 01:04:03PM -0700, Yu Zhao wrote: > On Mon, Nov 23, 2020 at 06:35:55PM +0000, Will Deacon wrote: > > On Fri, Nov 20, 2020 at 01:40:05PM -0700, Yu Zhao wrote: > > > On Fri, Nov 20, 2020 at 02:35:57PM +0000, Will Deacon wrote: > > > > clear_refs_write() uses the 'fullmm' API for invalidating TLBs after > > > > updating the page-tables for the current mm. However, since the mm is not > > > > being freed, this can result in stale TLB entries on architectures which > > > > elide 'fullmm' invalidation. > > > > > > > > Ensure that TLB invalidation is performed after updating soft-dirty > > > > entries via clear_refs_write() by using the non-fullmm API to MMU gather. > > > > > > > > Signed-off-by: Will Deacon > > > > --- > > > > fs/proc/task_mmu.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > > > > index a76d339b5754..316af047f1aa 100644 > > > > --- a/fs/proc/task_mmu.c > > > > +++ b/fs/proc/task_mmu.c > > > > @@ -1238,7 +1238,7 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf, > > > > count = -EINTR; > > > > goto out_mm; > > > > } > > > > - tlb_gather_mmu_fullmm(&tlb, mm); > > > > + tlb_gather_mmu(&tlb, mm, 0, TASK_SIZE); > > > > > > Let's assume my reply to patch 4 is wrong, and therefore we still need > > > tlb_gather/finish_mmu() here. But then wouldn't this change deprive > > > architectures other than ARM the opportunity to optimize based on the > > > fact it's a full-mm flush? > > I double checked my conclusion on patch 4, and aside from a couple > of typos, it still seems correct after the weekend. I still need to digest that, but I would prefer that we restore the invalidation first, and then have a subsequent commit to relax it. I find it hard to believe that the behaviour in mainline at the moment is deliberate. That is, I'm not against optimising this, but I'd rather get it "obviously correct" first and the current code is definitely not that. > > Only for the soft-dirty case, but I think TLB invalidation is required > > there because we are write-protecting the entries and I don't see any > > mechanism to handle lazy invalidation for that (compared with the aging > > case, which is handled via pte_accessible()). > > The lazy invalidation for that is done when we write-protect a page, > not an individual PTE. When we do so, our decision is based on both > the dirty bit and the writable bit on each PTE mapping this page. So > we only need to make sure we don't lose both on a PTE. And we don't > here. Sorry, I don't follow what you're getting at here (page vs pte). Please can you point me to the code you're referring to? The case I'm worried about is code that holds sufficient locks (e.g. mmap_sem + ptl) finding an entry where !pte_write() and assuming (despite pte_dirty()) that there can't be any concurrent modifications to the mapped page. Granted, I haven't found anything doing that, but I could not convince myself that it would be a bug to write such code, either. > > Furthermore, If we decide that we can relax the TLB invalidation > > requirements here, then I'd much rather than was done deliberately, rather > > than as an accidental side-effect of another commit (since I think the > > current behaviour was a consequence of 7a30df49f63a). > > Nope. tlb_gather/finish_mmu() should be added by b3a81d0841a9 > ("mm: fix KSM data corruption") in the first place. Sure, but if you check out b3a81d0841a9 then you have a fullmm TLB invalidation in tlb_finish_mmu(). 7a30df49f63a is what removed that, no? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel