lkml.org 
[lkml]   [2010]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] mm,migration: During fork(), wait for migration to end if migration PTE is encountered
On Wed, 28 Apr 2010 00:22:45 +0200
Andrea Arcangeli <aarcange@redhat.com> wrote:

> Ok I had a first look:
>
> On Tue, Apr 27, 2010 at 10:30:50PM +0100, Mel Gorman wrote:
> > CPUA CPU B
> > do_fork()
> > copy_mm() (from process 1 to process2)
> > insert new vma to mmap_list (if inode/anon_vma)
>
> Insert to the tail of the anon_vma list...
>
> > pte_lock(process1)
> > unmap a page
> > insert migration_entry
> > pte_unlock(process1)
> >
> > migrate page copy
> > copy_page_range
> > remap new page by rmap_walk()
>
> rmap_walk will walk process1 first! It's at the head, the vmas with
> unmapped ptes are at the tail so process1 is walked before process2.
>
> > pte_lock(process2)
> > found no pte.
> > pte_unlock(process2)
> > pte lock(process2)
> > pte lock(process1)
> > copy migration entry to process2
> > pte unlock(process1)
> > pte unlokc(process2)
> > pte_lock(process1)
> > replace migration entry
> > to new page's pte.
> > pte_unlock(process1)
>
> rmap_walk has to lock down process1 before process2, this is the
> ordering issue I already mentioned in earlier email. So it cannot
> happen and this patch is unnecessary.
>
> The ordering is fundamental and as said anon_vma_link already adds new
> vmas to the _tail_ of the anon-vma. And this is why it has to add to
> the tail. If anon_vma_link would add new vmas to the head of the list,
> the above bug could materialize, but it doesn't so it cannot happen.
>
> In mainline anon_vma_link is called anon_vma_chain_link, see the
> list_add_tail there to provide this guarantee.
>
> Because process1 is walked first by CPU A, the migration entry is
> replaced by the final pte before copy-migration-entry
> runs. Alternatively if copy-migration-entry runs before before
> process1 is walked, the migration entry will be copied and found in
> process 2.
>

I already explained this doesn't happend and said "I'm sorry".

But considering maintainance, it's not necessary to copy migration ptes
and we don't have to keep a fundamental risks of migration circus.

So, I don't say "we don't need this patch."

Thanks,
-Kame



\
 
 \ /
  Last update: 2010-04-28 01:59    [W:0.569 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site