On Fri, Aug 7, 2020 at 9:34 PM Linus Torvalds torvalds@linux-foundation.org wrote:
On Fri, Aug 7, 2020 at 1:53 PM Yang Shi shy828301@gmail.com 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