Re: [PATCH] ppc64: Fix possible race with set_pte on a present PTE

[Andrea Arcangeli <andrea@suse.de>]

On Tue, May 25, 2004 at 02:43:46PM -0700, Linus Torvalds wrote:
> 
> 
> On Tue, 25 May 2004, Andrea Arcangeli wrote:
> > 
> > 			entry = ptep_get_and_clear(pte);
> > 			set_pte(pte, pte_modify(entry, newprot));
> > 
> > Again no atomic instructions.
> 
> Well, actually, that "ptep_get_and_clear()" is actually an atomic 
> instruction. Or at least it had _better_ be.

sure, I really meant no "new" atomic instruction.

> > I believe using ptep_establish in handle_mm_fault makes little sense,
> > to make the most obvious example there's no need of flushing the tlb in
> > handle_mm_fault to resolve young or dirty page faults. Not a big deal
> > for x86 and x86-64 that reaches that path only if a race happens, but on
> > alpha we shouldn't flush the tlb. If some weird architecture really need
> > to flush the tlb they still can inside
> > ptep_handle_[young|dirty]_page_fault.
> 
> Actually, especially on alpha we _do_ need to flush the TLB.
> 
> Think about it: the TLB clearly contains the right virt->physical mapping, 
> but the TLB contains the wrong "control bits". 
> 
> If we don't flush the TLB, the TLB will _continue_ to contain the wrong 
> control bits.

I expected the pal code to re-read the pte if the control bits asked for
page fault, like it must happen if the control bits are set to
non-present. This latter this must be true or linux wouldn't run at all
on alpha.  We certainly don't flush the tlb after marking the page from
non-present to present, example in do_anonymous_page. Anyways if the pal
code behaves like that specifically with the KWE/UWE/KRE/URE and not
with the PAGE_VALID bit, I obviously agree have to flush the tlb. I just
didn't expect it, though I admit I couldn't easily find specs about it.

> And as you saw earlier, if those bits are wrong, you get some really nasty 
> behaviour, like infinite page faults. If the TLB still says that the page 
> is non-readable, even though _memory_ says it is readable, it doesn't much 
> matter that we updated the page tables correctly in memory. The CPU will 
> use the TLB.
> 
> So that TLB flush actually _is_ fundamental. 
> 
> Arguably we could remove it from x86. On the other hand, arguably it 
> doesn't actually matter on x86, so..

it doesn't matter in handle_mm_fault but it does matter in do_wp_page.
since we've to mess with ptep_establish to fix this race, it would be
nice to remove such flush_tlb_page from do_wp_page. Or is the x86 tlb
also not refilling the pte automatically if the control bits asks for
page-fault? In mprotect obviously we've to flush since we can upgrade
and downgrade the control bits, but in do_wp_page we only ugprade, so
there should be no need of tlb flush. I'll try to find some
documentation about this to be sure, at least for x86 it should be easy
to find.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>