>
> Kanoj Sarcar wrote:
> >
> > >
> > > it seems we have a smp race in zap_page_range():
> > >
> > > When we remove a page from the page tables, we must call:
> > >
> > > flush_cache_page();
> > > pte_clear();
> > > flush_tlb_page();
> > > free_page();
> > >
> > > We must not free the page before we have called flush_tlb_xy(),
> > > otherwise the second cpu could access memory that already freed.
> > >
> > > but zap_page_range() calls free_page() before the flush_tlb() call.
> > >
> > > Is that really a bug, has anyone a good idea how to fix that?
> >
> > Why do you think this is a bug? After the pte_clear, we need to flush
> > tlb, so that if anyone wants to drag in the mapping (by accessing the
> > virtual address), he will fault (since translation is not in tlb) and
> > wait on mmap_sem.
>
> Because the second cpu can use a stale tlb entry. We must free the page
> _after_ the flush, not before the flush.
>
> Example:
> cpu1, cpu2: execute 2 threads from one process.
> cpu3: unrelated thread that allocates a new page.
>
> cpu1:
> 1) writes to one page in a tight loop. The tlb entry won't be discared
> by the cpu without an explicit flush.
>
> cpu2:
> 2) sys_munmap()
> * zap_page_range(): calls free_page() for each page in the area.
> do_munmap() for a 500 MB block will take a few milliseconds.
>
> cpu3:
> 3) somewhere: get_free_page(). Now it gets a pointer to a page that cpu1
> still writes to.
>
> cpu2:
> 4) zap_page_range() returns, now the tlb is flushed.
>
> cpu1:
> 5) received the ipi, the local tlb is flushed, page fault.
> * but: cpu1 stomped on the page that was allocated by cpu3.
>
Yup, you got it, I had forgotten this issue. Btw, a bunch of people are
aware of the pte handling races in Linux (Linus, Alan, David M etc). In
fact, around 2.2.10 timeframe, Alan and I did a quick study of such races.
I posted a patch, we had some conversation, but ultimately we have not
fixed any of these races.
If you are interested, you can read a discussion and the patch at
http://reality.sgi.com/kanoj_engr/smppte.patch
The above race that you point out is covered in a somewhat different
fashion: "1. During munmap, clones should be restricted while the file
pages are being written out to disk, else some writes are not synced
to disk at unmap time."
>
> >
> > But a race does exist in establish_pte(), when the flush_tlb happens
> > _before_ the set_pte(), another thread might drag in the old translation
> > on a different cpu.
> >
>
> Yes, establish_pte() is broken. We should reverse the calls:
>
> set_pte(); /* update the kernel page tables */
> update_mmu(); /* update architecture specific page tables. */
> flush_tlb(); /* and flush the hardware tlb */
>
People are aware of this too, it was introduced during the 390 merge.
I tried talking to the IBM guy about this, I didn't see a response from
him ...
I think what we now need is a critical mass, something that will make us
go "okay, lets just fix these races once and for all".
>
> > >
> > > filemap_sync() calls flush_tlb_page() for each page, but IMHO this is a
> > > really bad idea, the performance will suck with multi-threaded apps on
> > > SMP.
> >
> > The best you can do probably is a flush_tlb_range?
>
> filemap_sync() calls both flush_tlb_range() and flush_tlb_page() on each
> page, I just don't know which call I should remove :-/
Hmm, this is related to the above race I quoted... see the patch in this
regard.
Kanoj
>
> Btw, I couldn't find an update_mmu_range() function, what's the exact
> purpose of update_mmu_cache()? mips64 uses this function.
>
> Can't we merge update_mmu_cache() with flush_tlb_page()?
>
> --
> Manfred
>
> --
> 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.eu.org/Linux-MM/
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Apr 15 2000 - 21:00:12 EST