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=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED 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 42215C56202 for ; Wed, 25 Nov 2020 22:52:55 +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 B0BB72068D for ; Wed, 25 Nov 2020 22:52:54 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="dUAmbKDZ"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="MQ5Xh/PG" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B0BB72068D 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=CWc95eXWPbjbEJ/Tx17swr/vAq+8LojGdsow+hUVyd4=; b=dUAmbKDZchZtV1s0PNuoNtbRG CfLQXOUt8COO4hO8f0GFgmUikXI73w5xUsCVjinu1wnMzB7dQmWJ3TP7CFqS6AxjocznV+NaIYVyX jcZMpHSxCZ8h+uc6ahQJouNXVvKLt5Mwon/NA9EgyzJYLubuEKRC+dnko4S9v2M2qvZCfPN5y4Lti D9nXwdtm14wEsHFe3369Sm9uWK3EflyEvP0Q3m0v4atLEDNB/l2ec5kMtJRrVoAHrORAZFG9nIYIg mkfZw50vMZVLjkmzTU0zbDsekcWWHnICle9fCSbANh4/eu5Oya4qizpoGDtITW+7xsRHfXcP1QVkP rB5IYrN+w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1ki3dN-0006hh-VR; Wed, 25 Nov 2020 22:51:34 +0000 Received: from mail-pf1-x443.google.com ([2607:f8b0:4864:20::443]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1ki3dK-0006g7-Iz for linux-arm-kernel@lists.infradead.org; Wed, 25 Nov 2020 22:51:31 +0000 Received: by mail-pf1-x443.google.com with SMTP id w187so3740095pfd.5 for ; Wed, 25 Nov 2020 14:51:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=IdH7TWjWbi5QdZ7NXo5aR6u+Q2zXh+GO9Yj+ORgSPoY=; b=MQ5Xh/PGhwHXhRhZhdNCxder7xSTQ9c8XILgpk60HKNuW8sNh+UTaqxTpHppOOlVWQ LEk5KNshPoI19tYCSjMMkpYh8IxFwqlRSbF7uG4tFoHUCZlILjLHH19JlRqcqVt5U9to r3YgaI2G9fvEGBj5D/6hKThlXZkWyM6HcpyA3AUVHPdvfDzIUdIuwfg5m6VeF8BrXap4 IFiwRQHhgLeiZYuT9Em307w5xlOGeCQMhJarZRfB/PCHv/iE3IPgZov+7Vuo8j8644GA NSKx8d2/AHLcc/Qq6HqYA+TLffx7tlNMv6uaxXZLTLnQW5EzYbUrUHqyi842+5cJ3aRr 0n1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=IdH7TWjWbi5QdZ7NXo5aR6u+Q2zXh+GO9Yj+ORgSPoY=; b=m0AIWsAxwIaX90J2yOb0pFUZG5GAb6HV7kPzMOgppF7I52MijE71SPeZRaHODqZ8rC CeREAScMefArJ5CcaTa6QzSYqVK191EzvfjJjJ0tcQX09YnzT1mr/uDQQLYLej5WtXEH Kd7NclLyzDwXuFzIGIhkcj6GmUosIxKPfQf5SZnKHj9zUAR6wUB/f2JbObk1zkkcwTqN QBqf1v7WJ7/yPdkjlv08desOJILh+QArr1ZCs6bL1JzT/ojIDCoDJjSWv5p/CPi+5pLa RQEj23f8eD2kyIw4c7lUZ4zq5A6dCSQ+JLfq6DZCRp4wNL0vjeP/2vleyFV1A96yZu3n JK2g== X-Gm-Message-State: AOAM533qPyoFGXGXRklBtjZK0kqvQPP9FemTUPAPVbLYDI+AbIFzpMF4 ZGIe36s17DxH8LD4jRWmLQY= X-Google-Smtp-Source: ABdhPJxsXWrFiRFquDGdwhYbPpoLwY1x6gikdnoflq4amxZ0nT7ZkLwmnkYhG2jA5kMJdJ0EZF4WkQ== X-Received: by 2002:a17:90a:f0cd:: with SMTP id fa13mr14046pjb.118.1606344688500; Wed, 25 Nov 2020 14:51:28 -0800 (PST) Received: from google.com ([2620:15c:211:201:7220:84ff:fe09:5e58]) by smtp.gmail.com with ESMTPSA id v3sm2759284pfn.215.2020.11.25.14.51.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Nov 2020 14:51:27 -0800 (PST) Date: Wed, 25 Nov 2020 14:51:25 -0800 From: Minchan Kim To: Will Deacon Subject: Re: [PATCH 4/6] mm: proc: Invalidate TLB after clearing soft-dirty page state Message-ID: <20201125225125.GE1484898@google.com> References: <20201120143557.6715-1-will@kernel.org> <20201120143557.6715-5-will@kernel.org> <20201120150023.GH3040@hirez.programming.kicks-ass.net> <20201120155514.GA3377168@google.com> <20201123184113.GD11688@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201123184113.GD11688@willie-the-truck> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201125_175130_691536_FABA9E87 X-CRM114-Status: GOOD ( 30.60 ) 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, Yu Zhao , Anshuman Khandual , Peter Zijlstra , Catalin Marinas , linux-kernel@vger.kernel.org, linux-mm@kvack.org, 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 06:41:14PM +0000, Will Deacon wrote: > On Fri, Nov 20, 2020 at 07:55:14AM -0800, Minchan Kim wrote: > > On Fri, Nov 20, 2020 at 04:00:23PM +0100, Peter Zijlstra wrote: > > > On Fri, Nov 20, 2020 at 02:35:55PM +0000, Will Deacon wrote: > > > > Since commit 0758cd830494 ("asm-generic/tlb: avoid potential double flush"), > > > > TLB invalidation is elided in tlb_finish_mmu() if no entries were batched > > > > via the tlb_remove_*() functions. Consequently, the page-table modifications > > > > performed by clear_refs_write() in response to a write to > > > > /proc//clear_refs do not perform TLB invalidation. Although this is > > > > fine when simply aging the ptes, in the case of clearing the "soft-dirty" > > > > state we can end up with entries where pte_write() is false, yet a > > > > writable mapping remains in the TLB. > > > > > > > > Fix this by calling tlb_remove_tlb_entry() for each entry being > > > > write-protected when cleating soft-dirty. > > > > > > > > > > > @@ -1053,6 +1054,7 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma, > > > > ptent = pte_wrprotect(old_pte); > > > > ptent = pte_clear_soft_dirty(ptent); > > > > ptep_modify_prot_commit(vma, addr, pte, old_pte, ptent); > > > > + tlb_remove_tlb_entry(tlb, pte, addr); > > > > } else if (is_swap_pte(ptent)) { > > > > ptent = pte_swp_clear_soft_dirty(ptent); > > > > set_pte_at(vma->vm_mm, addr, pte, ptent); > > > > > > Oh! > > > > > > Yesterday when you had me look at this code; I figured the sane thing > > > to do was to make it look more like mprotect(). > > > > > > Why did you chose to make it work with mmu_gather instead? I'll grant > > > you that it's probably the smaller patch, but I still think it's weird > > > to use mmu_gather here. > > > > I agree. The reason why clear_refs_write used the gather API was [1] and > > seems like to overkill to me. > > I don't see why it's overkill. Prior to that commit, it called > flush_tlb_mm() directly. The TLB gather was added for increasing tlb flush pending count for stability bug, not for performance optimiataion(The commit never had any number to support it and didn't have the logic to handle each pte with tlb gather) and then it introduced a bug now so I take it as overkill since it made complication from the beginning *unnecessary*. > > > We could just do like [inc|dec]_tlb_flush_pending with flush_tlb_mm at > > right before dec_tlb_flush_pending instead of gather. > > > > thought? > > I'm not sure why this is better; it's different to the madvise() path, and > will need special logic to avoid the flush in the case where we're just > doing aging. I thought it's better to fix the bug first as *simple* patch and then do optimization based on it. Anyway, following to Yu's comment, we don't need gather API and even the flush if we give up the accuarcy(but I want to have it). _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel