Re: [patch 001/163] mm/memory.c: avoid access flag update TLB flush for retried page fault

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux