From: Will Deacon <will@kernel.org> To: Yu Zhao <yuzhao@google.com> Cc: linux-kernel@vger.kernel.org, kernel-team@android.com, Catalin Marinas <catalin.marinas@arm.com>, Minchan Kim <minchan@kernel.org>, Peter Zijlstra <peterz@infradead.org>, Linus Torvalds <torvalds@linux-foundation.org>, Anshuman Khandual <anshuman.khandual@arm.com>, linux-mm@kvack.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 6/6] mm: proc: Avoid fullmm flush for young/dirty bit toggling Date: Mon, 23 Nov 2020 18:35:55 +0000 [thread overview] Message-ID: <20201123183554.GC11688@willie-the-truck> (raw) In-Reply-To: <20201120204005.GC1303870@google.com> 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 <will@kernel.org> > > --- > > 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? 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()). 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). > It seems to me ARM's interpretation of tlb->fullmm is a special case, > not the other way around. Although I agree that this is subtle and error-prone (which is why I'm trying to make the API more explicit here), it _is_ documented clearly in asm-generic/tlb.h: * - mmu_gather::fullmm * * A flag set by tlb_gather_mmu() to indicate we're going to free * the entire mm; this allows a number of optimizations. * * - We can ignore tlb_{start,end}_vma(); because we don't * care about ranges. Everything will be shot down. * * - (RISC) architectures that use ASIDs can cycle to a new ASID * and delay the invalidation until ASID space runs out. Will
WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will@kernel.org> To: Yu Zhao <yuzhao@google.com> Cc: kernel-team@android.com, Anshuman Khandual <anshuman.khandual@arm.com>, Peter Zijlstra <peterz@infradead.org>, Catalin Marinas <catalin.marinas@arm.com>, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Minchan Kim <minchan@kernel.org>, Linus Torvalds <torvalds@linux-foundation.org>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 6/6] mm: proc: Avoid fullmm flush for young/dirty bit toggling Date: Mon, 23 Nov 2020 18:35:55 +0000 [thread overview] Message-ID: <20201123183554.GC11688@willie-the-truck> (raw) In-Reply-To: <20201120204005.GC1303870@google.com> 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 <will@kernel.org> > > --- > > 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? 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()). 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). > It seems to me ARM's interpretation of tlb->fullmm is a special case, > not the other way around. Although I agree that this is subtle and error-prone (which is why I'm trying to make the API more explicit here), it _is_ documented clearly in asm-generic/tlb.h: * - mmu_gather::fullmm * * A flag set by tlb_gather_mmu() to indicate we're going to free * the entire mm; this allows a number of optimizations. * * - We can ignore tlb_{start,end}_vma(); because we don't * care about ranges. Everything will be shot down. * * - (RISC) architectures that use ASIDs can cycle to a new ASID * and delay the invalidation until ASID space runs out. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-11-23 18:36 UTC|newest] Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-20 14:35 [PATCH 0/6] tlb: Fix access and (soft-)dirty bit management Will Deacon 2020-11-20 14:35 ` Will Deacon 2020-11-20 14:35 ` [PATCH 1/6] arm64: pgtable: Fix pte_accessible() Will Deacon 2020-11-20 14:35 ` Will Deacon 2020-11-20 16:03 ` Minchan Kim 2020-11-20 16:03 ` Minchan Kim 2020-11-20 19:53 ` Yu Zhao 2020-11-20 19:53 ` Yu Zhao 2020-11-23 13:27 ` Catalin Marinas 2020-11-23 13:27 ` Catalin Marinas 2020-11-24 10:02 ` Anshuman Khandual 2020-11-24 10:02 ` Anshuman Khandual 2020-11-20 14:35 ` [PATCH 2/6] arm64: pgtable: Ensure dirty bit is preserved across pte_wrprotect() Will Deacon 2020-11-20 14:35 ` Will Deacon 2020-11-20 17:09 ` Minchan Kim 2020-11-20 17:09 ` Minchan Kim 2020-11-23 14:31 ` Catalin Marinas 2020-11-23 14:31 ` Catalin Marinas 2020-11-23 14:22 ` Catalin Marinas 2020-11-23 14:22 ` Catalin Marinas 2020-11-20 14:35 ` [PATCH 3/6] tlb: mmu_gather: Remove unused start/end arguments from tlb_finish_mmu() Will Deacon 2020-11-20 14:35 ` Will Deacon 2020-11-20 17:20 ` Linus Torvalds 2020-11-20 17:20 ` Linus Torvalds 2020-11-20 17:20 ` Linus Torvalds 2020-11-23 16:48 ` Will Deacon 2020-11-23 16:48 ` Will Deacon 2020-11-20 14:35 ` [PATCH 4/6] mm: proc: Invalidate TLB after clearing soft-dirty page state Will Deacon 2020-11-20 14:35 ` Will Deacon 2020-11-20 15:00 ` Peter Zijlstra 2020-11-20 15:00 ` Peter Zijlstra 2020-11-20 15:09 ` Peter Zijlstra 2020-11-20 15:09 ` Peter Zijlstra 2020-11-20 15:15 ` Will Deacon 2020-11-20 15:15 ` Will Deacon 2020-11-20 15:27 ` Peter Zijlstra 2020-11-20 15:27 ` Peter Zijlstra 2020-11-23 18:23 ` Will Deacon 2020-11-23 18:23 ` Will Deacon 2020-11-20 15:55 ` Minchan Kim 2020-11-20 15:55 ` Minchan Kim 2020-11-23 18:41 ` Will Deacon 2020-11-23 18:41 ` Will Deacon 2020-11-25 22:51 ` Minchan Kim 2020-11-25 22:51 ` Minchan Kim 2020-11-20 20:22 ` Yu Zhao 2020-11-20 20:22 ` Yu Zhao 2020-11-21 2:49 ` Yu Zhao 2020-11-21 2:49 ` Yu Zhao 2020-11-23 19:21 ` Yu Zhao 2020-11-23 19:21 ` Yu Zhao 2020-11-23 22:04 ` Will Deacon 2020-11-23 22:04 ` Will Deacon 2020-11-20 14:35 ` [PATCH 5/6] tlb: mmu_gather: Introduce tlb_gather_mmu_fullmm() Will Deacon 2020-11-20 14:35 ` Will Deacon 2020-11-20 17:22 ` Linus Torvalds 2020-11-20 17:22 ` Linus Torvalds 2020-11-20 17:22 ` Linus Torvalds 2020-11-20 17:31 ` Linus Torvalds 2020-11-20 17:31 ` Linus Torvalds 2020-11-20 17:31 ` Linus Torvalds 2020-11-23 16:48 ` Will Deacon 2020-11-23 16:48 ` Will Deacon 2021-02-01 11:32 ` [tip: core/mm] tlb: mmu_gather: Remove start/end arguments from tlb_gather_mmu() tip-bot2 for Will Deacon 2020-11-22 15:11 ` [tlb] e242a269fa: WARNING:at_mm/mmu_gather.c:#tlb_gather_mmu kernel test robot 2020-11-23 17:51 ` Will Deacon 2020-11-23 17:51 ` Will Deacon 2020-11-20 14:35 ` [PATCH 6/6] mm: proc: Avoid fullmm flush for young/dirty bit toggling Will Deacon 2020-11-20 14:35 ` Will Deacon 2020-11-20 17:41 ` Linus Torvalds 2020-11-20 17:41 ` Linus Torvalds 2020-11-20 17:41 ` Linus Torvalds 2020-11-20 17:45 ` Linus Torvalds 2020-11-20 17:45 ` Linus Torvalds 2020-11-20 17:45 ` Linus Torvalds 2020-11-20 20:40 ` Yu Zhao 2020-11-20 20:40 ` Yu Zhao 2020-11-23 18:35 ` Will Deacon [this message] 2020-11-23 18:35 ` Will Deacon 2020-11-23 20:04 ` Yu Zhao 2020-11-23 20:04 ` Yu Zhao 2020-11-23 21:17 ` Will Deacon 2020-11-23 21:17 ` Will Deacon 2020-11-24 1:13 ` Yu Zhao 2020-11-24 1:13 ` Yu Zhao 2020-11-24 14:31 ` Will Deacon 2020-11-24 14:31 ` Will Deacon 2020-11-25 22:01 ` Minchan Kim 2020-11-25 22:01 ` Minchan Kim 2020-11-24 14:46 ` Peter Zijlstra 2020-11-24 14:46 ` Peter Zijlstra
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20201123183554.GC11688@willie-the-truck \ --to=will@kernel.org \ --cc=anshuman.khandual@arm.com \ --cc=catalin.marinas@arm.com \ --cc=kernel-team@android.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=minchan@kernel.org \ --cc=peterz@infradead.org \ --cc=torvalds@linux-foundation.org \ --cc=yuzhao@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.