| Message ID | 20200116064531.483522-4-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixup page directory freeing | expand |
On 1/16/20 12:15 PM, Aneesh Kumar K.V wrote: > From: Peter Zijlstra <peterz@infradead.org> > > Aneesh reported that: > > tlb_flush_mmu() > tlb_flush_mmu_tlbonly() > tlb_flush() <-- #1 > tlb_flush_mmu_free() > tlb_table_flush() > tlb_table_invalidate() > tlb_flush_mmu_tlbonly() > tlb_flush() <-- #2 > > does two TLBIs when tlb->fullmm, because __tlb_reset_range() will not > clear tlb->end in that case. > > Observe that any caller to __tlb_adjust_range() also sets at least one > of the tlb->freed_tables || tlb->cleared_p* bits, and those are > unconditionally cleared by __tlb_reset_range(). > > Change the condition for actually issuing TLBI to having one of those > bits set, as opposed to having tlb->end != 0. > We should possibly get this to stable too along with the first two patches. I am not quiet sure if this will qualify for a stable backport. Hence avoided adding Cc:stable@kernel.org > Reported-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > --- > include/asm-generic/tlb.h | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > index 9e22ac369d1d..b36b3bef5661 100644 > --- a/include/asm-generic/tlb.h > +++ b/include/asm-generic/tlb.h > @@ -402,7 +402,12 @@ tlb_update_vma_flags(struct mmu_gather *tlb, struct vm_area_struct *vma) { } > > static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb) > { > - if (!tlb->end) > + /* > + * Anything calling __tlb_adjust_range() also sets at least one of > + * these bits. > + */ > + if (!(tlb->freed_tables || tlb->cleared_ptes || tlb->cleared_pmds || > + tlb->cleared_puds || tlb->cleared_p4ds)) > return; > > tlb_flush(tlb); >
On Thu, 16 Jan 2020 12:19:59 +0530 "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> wrote: > On 1/16/20 12:15 PM, Aneesh Kumar K.V wrote: > > From: Peter Zijlstra <peterz@infradead.org> > > > > Aneesh reported that: > > > > tlb_flush_mmu() > > tlb_flush_mmu_tlbonly() > > tlb_flush() <-- #1 > > tlb_flush_mmu_free() > > tlb_table_flush() > > tlb_table_invalidate() > > tlb_flush_mmu_tlbonly() > > tlb_flush() <-- #2 > > > > does two TLBIs when tlb->fullmm, because __tlb_reset_range() will not > > clear tlb->end in that case. > > > > Observe that any caller to __tlb_adjust_range() also sets at least one > > of the tlb->freed_tables || tlb->cleared_p* bits, and those are > > unconditionally cleared by __tlb_reset_range(). > > > > Change the condition for actually issuing TLBI to having one of those > > bits set, as opposed to having tlb->end != 0. > > > > > We should possibly get this to stable too along with the first two > patches. I am not quiet sure if this will qualify for a stable backport. > Hence avoided adding Cc:stable@kernel.org I'm not seeing any description of the user-visible runtime effects. Always needed, especially for -stable, please. It appears to be a small performance benefit? If that benefit was "large" and measurements were presented then that would be something we might wish to backport.
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h index 9e22ac369d1d..b36b3bef5661 100644 --- a/include/asm-generic/tlb.h +++ b/include/asm-generic/tlb.h @@ -402,7 +402,12 @@ tlb_update_vma_flags(struct mmu_gather *tlb, struct vm_area_struct *vma) { } static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb) { - if (!tlb->end) + /* + * Anything calling __tlb_adjust_range() also sets at least one of + * these bits. + */ + if (!(tlb->freed_tables || tlb->cleared_ptes || tlb->cleared_pmds || + tlb->cleared_puds || tlb->cleared_p4ds)) return; tlb_flush(tlb);