lkml.org 
[lkml]   [2006]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 5/5] Swapless V2: Revise main migration logic
On Mon, 17 Apr 2006 20:16:11 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> wrote:

> On Tue, 18 Apr 2006, KAMEZAWA Hiroyuki wrote:
>
> > I think following patch will help. but this increases complexity...
>
> Hmm... So the idea is to lock the anon vma before removing the ptes and
> keep it until we are finished migrating. I like it! That would also reduce
> the locking overhead.
>
> > /*
> > + * When mmap->sem is not held, we have to guarantee anon_vma is not freed.
> > + */
> > +static void migrate_lock_anon_vma(struct page *page)
> > +{
> > + unsigned long mapping;
> > + struct anon_vma *anon_vma;
> > + struct vm_area_struct *vma;
> > +
> > + if (PageAnon(page))
> > + page_lock_anon_vma(page);
> > + /* remove migration ptes will unlock */
> > +}
>
> We need a whole function for two statements?
>
Ah, ok, inlining is better.


> > */
> > anon_vma = (struct anon_vma *) (mapping - PAGE_MAPPING_ANON);
> > - spin_lock(&anon_vma->lock);
>
> Maybe we better pass the anon_vma as a parameter?
>
Agreed, it will improve total look.

> > +++ Christoph-NewMigrationV2/mm/rmap.c
> > @@ -160,7 +160,7 @@ void anon_vma_unlink(struct vm_area_stru
> > empty = list_empty(&anon_vma->head);
> > spin_unlock(&anon_vma->lock);
> >
> > - if (empty)
> > + if (empty && !anon_vma->async_refernece)
> > anon_vma_free(anon_vma);
> > }
>
> async_reference? What is this for? This does not exist in Linus'
> tree.
please ignore ;), this is trash of my old tree. sorry.

Here is updated one.
-Kame

hold anon_vma->lock under migration.

While migration, page_mapcount(page) goes down to 0 and page->mapping is valid.
This breaks assumptions around page_mapcount() and page->mapping.
(See rmap.c, page_remove_rmap())
If mmap->sem is held while migration, there is no problem. But if mmap->sem is
not held, this is a race.

This patch locks anon_vma under migration.

Signed-Off-By: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Index: Christoph-NewMigrationV2/mm/migrate.c
===================================================================
--- Christoph-NewMigrationV2.orig/mm/migrate.c
+++ Christoph-NewMigrationV2/mm/migrate.c
@@ -184,28 +184,18 @@ out:
* Must hold mmap_sem lock on at least one of the vmas containing
* the page so that the anon_vma cannot vanish.
*/
-static void remove_migration_ptes(struct page *old, struct page *new)
+static void remove_migration_ptes(struct anon_vma *anon_vma,
+ struct page *old, struct page *new)
{
- struct anon_vma *anon_vma;
struct vm_area_struct *vma;
unsigned long mapping;

- mapping = (unsigned long)new->mapping;
-
- if (!mapping || (mapping & PAGE_MAPPING_ANON) == 0)
+ if (!anon_vma)
return;

- /*
- * We hold the mmap_sem lock. So no need to call page_lock_anon_vma.
- */
- anon_vma = (struct anon_vma *) (mapping - PAGE_MAPPING_ANON);
- spin_lock(&anon_vma->lock);
-
list_for_each_entry(vma, &anon_vma->head, anon_vma_node)
remove_migration_pte(vma, page_address_in_vma(new, vma),
old, new);
-
- spin_unlock(&anon_vma->lock);
}

/*
@@ -368,20 +358,24 @@ EXPORT_SYMBOL(migrate_page_copy);
int migrate_page(struct page *newpage, struct page *page)
{
int rc;
-
+ struct anon_vma *anon_vma;
BUG_ON(PageWriteback(page)); /* Writeback must be complete */
-
+ if (PageAnon(page)) {
+ anon_vma = page_lock_anon_vma(page);
+ }
rc = migrate_page_remove_references(newpage, page,
page_mapping(page) ? 2 : 1);

if (rc) {
- remove_migration_ptes(page, page);
- return rc;
+ remove_migration_ptes(anon_vma, page, page);
+ goto unlock_out;
}
-
migrate_page_copy(newpage, page);
- remove_migration_ptes(page, newpage);
- return 0;
+ remove_migration_ptes(anon_vma, page, newpage);
+unlock_out:
+ if (anon_vma)
+ spin_unlock(&anon_vma->lock);
+ return rc;
}
EXPORT_SYMBOL(migrate_page);

Index: Christoph-NewMigrationV2/mm/rmap.c
===================================================================
--- Christoph-NewMigrationV2.orig/mm/rmap.c
+++ Christoph-NewMigrationV2/mm/rmap.c
@@ -717,7 +717,13 @@ static int try_to_unmap_anon(struct page
struct vm_area_struct *vma;
int ret = SWAP_AGAIN;

- anon_vma = page_lock_anon_vma(page);
+ if (migration) { /* anon_vma->lock is held under migration */
+ unsigned long mapping;
+ mapping = (unsigned long)page->mapping - PAGE_MAPPING_ANON;
+ anon_vma = (struct anon_vma *)mapping;
+ } else {
+ anon_vma = page_lock_anon_vma(page);
+ }
if (!anon_vma)
return ret;

@@ -726,7 +732,8 @@ static int try_to_unmap_anon(struct page
if (ret == SWAP_FAIL || !page_mapped(page))
break;
}
- spin_unlock(&anon_vma->lock);
+ if (!migration)
+ spin_unlock(&anon_vma->lock);
return ret;
}


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-04-18 05:34    [W:0.075 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site