On Mon, 17 Apr 2006 20:16:11 -0700 (PDT)
Christoph Lameter <[email protected]> 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 <[email protected]>
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
