Re: [PATCH 04/14] mm,migration: Allow the migration of PageSwapCache pages

[Mel Gorman <mel@csn.ul.ie>]

On Fri, Apr 23, 2010 at 08:52:03AM +0900, KAMEZAWA Hiroyuki wrote:
> On Thu, 22 Apr 2010 14:40:46 -0500 (CDT)
> Christoph Lameter <cl@linux.com> wrote:
> 
> > On Thu, 22 Apr 2010, Mel Gorman wrote:
> > 
> > > vma_adjust() is updating anon VMA information without any locks taken.
> > > In constract, file-backed mappings use the i_mmap_lock. This lack of
> > > locking can result in races with page migration. During rmap_walk(),
> > > vma_address() can return -EFAULT for an address that will soon be valid.
> > > This leaves a dangling migration PTE behind which can later cause a
> > > BUG_ON to trigger when the page is faulted in.
> > 
> > Isnt this also a race with reclaim /  swap?
> > 
> Yes, it's also race in reclaim/swap ...
>   page_referenced()
>   try_to_unmap().
>   rmap_walk()  <==== we hit this case.
> 
> But above 2 are not considered to be critical.
> 
> I'm not sure how this race affect KSM.
> 

I'm not that familiar with KSM but took a look through. Mostly,
accessing the VMA is protected by the mmap_sem with the exception of
rmap_walk_ksm. It needs similar protection for accessing the VMA than
rmap_walk_anon does.

Specifically, this part

                list_for_each_entry(vmac, &anon_vma->head, same_anon_vma) {
                        vma = vmac->vma;
                        if (rmap_item->address < vma->vm_start ||
                            rmap_item->address >= vma->vm_end)
                                continue;

needs to acquire the vma->anon_vma lock if it differs or in your case
call something similar to vma_address_safe.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab