What archs need flush_tlb_page() in handle_pte_fault() ?

What archs need flush_tlb_page() in handle_pte_fault() ?

[Benjamin Herrenschmidt]

Heya !

In my page table accessor spring cleaning, one of my targets is
flush_tlb_page(). At this stage, it's only called by generic code in one
place (in addition to the asm-generic bits that use it to implement
missing accessors, but I'm taking care of those spearately) :

In handle_pte_fault(), when the PTE is present -and-
ptep_set_access_flags() returns false -and- it's a write fault, we do a
flush_tlb_page().

ptep_set_access_flags() returning false typically means we don't
actually need to call update_mmu_cache() and haven't updated the PTE.

Now, I would like to understand what archs actually need that. If we
have lazy _PAGE_DIRTY handling, then ptep_set_access_flags() would have
done the flush already. I can imagine people may want to avoid the SMP
IPI in that case and only lazily flush on that CPU but that doesn't seem
to be what i386 does today.

In any case, I believe that this flush could be moved to inside
ptep_set_access_flags() for archs that need it, thus totally removing
the else { ... } clause in handle_pte_fault(). Archs that want to be
smart can do a local flush inside ptep_set_access_flags() if !changed &&
dirty, it all gets under arch control, and that last flush_tlb_page()
can be removed from generic code.

Now, before I actually remove it, I need to understand what archs
actually -need- that flush, so I can move it to their respective
ptep_set_access_flags() implementations.

I don't see i386 needing it unless I missed something.

For now, I'll assume nobody needs it. So please tell me if your arch
does and I'll make sure my patch has it fixed up properly.

Thanks !
Ben.

Re: What archs need flush_tlb_page() in handle_pte_fault() ?

[Benjamin Herrenschmidt]

On Thu, 2007-08-02 at 11:37 +1000, Benjamin Herrenschmidt wrote:
> Heya !
> 
> In my page table accessor spring cleaning, one of my targets is
> flush_tlb_page(). At this stage, it's only called by generic code in one
> place (in addition to the asm-generic bits that use it to implement
> missing accessors, but I'm taking care of those spearately) :

 .../...

No reply, so I suppose I can rip it out ? :-)

Thus any reason why that patch wouln't fly ? (not for 2.6.23 of course)

This removes the last occurence of flush_tlb_page() from generic code,
thus making this hook now optional for architectures that don't use
the helpers in asm-generic/pgtable.h

I couldn't find a case where this is actually needed, but I may well
have missed something. If I did though, please tell me what. If an
architecture need that call for obscure reason, it could be simply
folded in that architecture's implementation ptep_set_access_flags().

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Index: linux-work/mm/memory.c
===================================================================
--- linux-work.orig/mm/memory.c	2007-08-06 16:32:12.000000000 +1000
+++ linux-work/mm/memory.c	2007-08-06 16:34:07.000000000 +1000
@@ -2609,15 +2609,6 @@ static inline int handle_pte_fault(struc
 	if (ptep_set_access_flags(vma, address, pte, entry, write_access)) {
 		update_mmu_cache(vma, address, entry);
 		lazy_mmu_prot_update(entry);
-	} else {
-		/*
-		 * This is needed only for protection faults but the arch code
-		 * is not yet telling us if this is a protection fault or not.
-		 * This still avoids useless tlb flushes for .text page faults
-		 * with threads.
-		 */
-		if (write_access)
-			flush_tlb_page(vma, address);
 	}
 unlock:
 	pte_unmap_unlock(pte, ptl);