Re: [RFC][PATCH 05/11] asm-generic/tlb: Provide generic tlb_flush

From: Peter Zijlstra
Date: Thu Sep 13 2018 - 10:06:38 EST


On Thu, Sep 13, 2018 at 03:09:47PM +0200, Jann Horn wrote:
> On Thu, Sep 13, 2018 at 3:01 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > Provide a generic tlb_flush() implementation that relies on
> > flush_tlb_range(). This is a little awkward because flush_tlb_range()
> > assumes a VMA for range invalidation, but we no longer have one.
> >
> > Audit of all flush_tlb_range() implementations shows only vma->vm_mm
> > and vma->vm_flags are used, and of the latter only VM_EXEC (I-TLB
> > invalidates) and VM_HUGETLB (large TLB invalidate) are used.
> >
> > Therefore, track VM_EXEC and VM_HUGETLB in two more bits, and create a
> > 'fake' VMA.
> >
> > This allows architectures that have a reasonably efficient
> > flush_tlb_range() to not require any additional effort.
> [...]
> > +#define tlb_flush tlb_flush
> > +static inline void tlb_flush(struct mmu_gather *tlb)
> > +{
> > + if (tlb->fullmm || tlb->need_flush_all) {
> > + flush_tlb_mm(tlb->mm);
> > + } else {
> > + struct vm_area_struct vma = {
> > + .vm_mm = tlb->mm,
> > + .vm_flags = tlb->vma_exec ? VM_EXEC : 0 |
> > + tlb->vma_huge ? VM_HUGETLB : 0,
>
> This looks wrong to me. Bitwise OR has higher precedence than the
> ternary operator, so I think this code is equivalent to:
>
> .vm_flags = tlb->vma_exec ? VM_EXEC : (0 | tlb->vma_huge) ? VM_HUGETLB : 0
>
> meaning that executable+huge mappings would only get VM_EXEC, but not
> VM_HUGETLB.

Bah. Fixed that. Thanks!

--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -309,8 +309,8 @@ static inline void tlb_flush(struct mmu_
} else {
struct vm_area_struct vma = {
.vm_mm = tlb->mm,
- .vm_flags = tlb->vma_exec ? VM_EXEC : 0 |
- tlb->vma_huge ? VM_HUGETLB : 0,
+ .vm_flags = (tlb->vma_exec ? VM_EXEC : 0) |
+ (tlb->vma_huge ? VM_HUGETLB : 0),
};

flush_tlb_range(&vma, tlb->start, tlb->end);