On Fri, Aug 7, 2020 at 9:34 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Aug 7, 2020 at 1:53 PM Yang Shi <shy828301@xxxxxxxxx> wrote:
> >
> > I'm supposed Catalin would submit his proposal (flush local TLB for
> > spurious TLB fault on ARM) for this specific regression per the
> > discussion, right?
>
> I think arm64 should do that regardless, yes.
>
> But I would also be ok with a version that does the FAULT_FLAG_TRIED
> testing, but does it only for that spurious TLB flushing.
>
> This "let's not update the page tables at all" is wrong, when the only
> problem was the TLB flushing.
>
> So changing the current (but quesitonable)
>
> if (vmf->flags & FAULT_FLAG_WRITE)
> flush_tlb_fix_spurious_fault(vmf->vma, vmf->address);
>
> to be
>
> if (vmf->flags & (FAULT_FLAG_WRITE | FAULT_FLAG_TRIED))
> flush_tlb_fix_spurious_fault(vmf->vma, vmf->address);
It looks the retried fault still flush TLB with this change.
Shouldn't we do something like this to skip spurious TLB flush:
@@ -4251,6 +4251,9 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf)
vmf->flags & FAULT_FLAG_WRITE)) {
update_mmu_cache(vmf->vma, vmf->address, vmf->pte);
} else {
+ if (vmf->flags & FAULT_FLAG_TRIED)
+ goto unlock;
+
/*
* This is needed only for protection faults but the arch code
* is not yet telling us if this is a protection fault or not.
>
> would be fine.
>
> But this patch that changes any semantics outside just the flushin gis
> a complete no-no.
>
> Linus
