Skip to contentSkip to conversation

migration cache, updated

PATCH Marcelo Tosatti 6 participants 44 messages Oct 25, 2004 – Feb 9, 2005 2 messages not in the archive

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

Hi,

This is an improved version of the migration cache patch - thanks to everyone who contributed - Hirokazu, Iwamoto, Dave, Hugh.

Identification of migration pages is now done with SwapCache bit and special swap type as suggested.

Also fixed a plethora of other bugs present in the last patch.

It now works fine - survives stress testing.

Hugh - I'm not sure about reusing swap allocation code for this anymore - it is designed to deal with storage (all the batching , extent handling, etc).

the idr code is smaller - it does only what we need here.

Have to update it to last -mhp version.

include/linux/mm.h +22 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/mm.h linux-2.6.9-rc2-mm4.build/include/linux/mm.h
@@ -251,6 +251,24 @@ extern int capture_page_range(unsigned l
  * files which need it (119 of them)
  */
 #include <linux/page-flags.h>
+#include <linux/swap.h>
+#include <linux/swapops.h> 
+
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 
 /*
  * Methods to modify the page usage count.
@@ -458,11 +476,14 @@ void page_address_init(void);
 #define PAGE_MAPPING_ANON	1
 
 extern struct address_space swapper_space;
+extern struct address_space migration_space;
 static inline struct address_space *page_mapping(struct page *page)
 {
 	struct address_space *mapping = page->mapping;
 
-	if (unlikely(PageSwapCache(page)))
+	if (unlikely(PageMigration(page)))
+		mapping = &migration_space;
+	else if (unlikely(PageSwapCache(page)))
 		mapping = &swapper_space;
 	else if (unlikely((unsigned long)mapping & PAGE_MAPPING_ANON))
 		mapping = NULL;
include/linux/swap.h +16 -0
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/swap.h linux-2.6.9-rc2-mm4.build/include/linux/swap.h
@@ -253,6 +253,7 @@ extern sector_t map_swap_page(struct swa
 extern struct swap_info_struct *get_swap_info_struct(unsigned);
 extern int can_share_swap_page(struct page *);
 extern int remove_exclusive_swap_page(struct page *);
+extern int migration_remove_entry(swp_entry_t);
 struct backing_dev_info;
 
 extern struct swap_list_t swap_list;
@@ -321,6 +322,21 @@ static inline swp_entry_t get_swap_page(
 #define grab_swap_token()  do { } while(0)
 #define has_swap_token(x) 0
 
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 #endif /* CONFIG_SWAP */
 #endif /* __KERNEL__*/
 #endif /* _LINUX_SWAP_H */
include/linux/swapops.h +25 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/swapops.h linux-2.6.9-rc2-mm4.build/include/linux/swapops.h
@@ -10,7 +10,9 @@
  * swp_entry_t's are *never* stored anywhere in their arch-dependent format.
  */
 #define SWP_TYPE_SHIFT(e)	(sizeof(e.val) * 8 - MAX_SWAPFILES_SHIFT)
-#define SWP_OFFSET_MASK(e)	((1UL << SWP_TYPE_SHIFT(e)) - 1)
+#define SWP_OFFSET_MASK(e)	((1UL << (SWP_TYPE_SHIFT(e))) - 1)
+
+#define MIGRATION_TYPE  (MAX_SWAPFILES - 1)
 
 /*
  * Store a type+offset into a swp_entry_t in an arch-independent format
@@ -30,8 +32,7 @@ static inline swp_entry_t swp_entry(unsi
  */
 static inline unsigned swp_type(swp_entry_t entry)
 {
-	return (entry.val >> SWP_TYPE_SHIFT(entry)) &
-			((1 << MAX_SWAPFILES_SHIFT) - 1);
+	return ((entry.val >> SWP_TYPE_SHIFT(entry)));
 }
 
 /*
@@ -68,3 +69,24 @@ static inline pte_t swp_entry_to_pte(swp
 	BUG_ON(pte_file(__swp_entry_to_pte(arch_entry)));
 	return __swp_entry_to_pte(arch_entry);
 }
+
+static inline int pte_is_migration(pte_t pte)
+{
+	unsigned long swp_type;
+	swp_entry_t arch_entry;
+
+	arch_entry = __pte_to_swp_entry(pte);
+
+	swp_type = __swp_type(arch_entry);
+
+	return swp_type == MIGRATION_TYPE;
+}
+
+static inline pte_t migration_entry_to_pte(swp_entry_t entry)
+{
+	swp_entry_t arch_entry;
+	
+	arch_entry = __swp_entry(MIGRATION_TYPE, swp_offset(entry));
+	return __swp_entry_to_pte(arch_entry);
+}
+
mm/fremap.c +8 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/fremap.c linux-2.6.9-rc2-mm4.build/mm/fremap.c
@@ -11,7 +11,6 @@
 #include <linux/file.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
-#include <linux/swapops.h>
 #include <linux/rmap.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
@@ -43,8 +42,14 @@ static inline void zap_pte(struct mm_str
 			}
 		}
 	} else {
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) { 
+				migration_remove_entry(swp_entry);
+			} else {
+				free_swap_and_cache(swp_entry);
+			}
+		}
 		pte_clear(ptep);
 	}
 }
mm/memory.c +26 -8
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/memory.c linux-2.6.9-rc2-mm4.build/mm/memory.c
@@ -53,7 +53,6 @@
 #include <asm/tlbflush.h>
 #include <asm/pgtable.h>
 
-#include <linux/swapops.h>
 #include <linux/elf.h>
 
 #ifndef CONFIG_DISCONTIGMEM
@@ -456,8 +455,13 @@ static void zap_pte_range(struct mmu_gat
 		 */
 		if (unlikely(details))
 			continue;
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) {
+				migration_remove_entry(swp_entry);
+			} else
+				free_swap_and_cache(swp_entry);
+		}
 		pte_clear(ptep);
 	}
 	pte_unmap(ptep-1);
@@ -1408,6 +1412,9 @@ static int do_swap_page(struct mm_struct
 	pte_unmap(page_table);
 	spin_unlock(&mm->page_table_lock);
 again:
+	if (pte_is_migration(orig_pte)) {
+		page = lookup_migration_cache(entry.val);
+	} else {
 	page = lookup_swap_cache(entry);
 	if (!page) {
  		swapin_readahead(entry, address, vma);
@@ -1433,15 +1440,22 @@ again:
 		inc_page_state(pgmajfault);
 		grab_swap_token();
 	}
-
 	mark_page_accessed(page);
 	lock_page(page);
 	if (!PageSwapCache(page)) {
+		/* hiro: add !PageMigration(page) here */
 		/* page-migration has occured */
 		unlock_page(page);
 		page_cache_release(page);
 		goto again;
 	}
+	}
+
+
+	if (pte_is_migration(orig_pte)) {
+		mark_page_accessed(page);
+		lock_page(page);
+	}
 
 	/*
 	 * Back out if somebody else faulted in this pte while we
@@ -1459,10 +1473,14 @@ again:
 	}
 
 	/* The page isn't present yet, go ahead with the fault. */
-		
-	swap_free(entry);
-	if (vm_swap_full())
-		remove_exclusive_swap_page(page);
+
+	if (!pte_is_migration(orig_pte)) {
+		swap_free(entry);
+		if (vm_swap_full())
+			remove_exclusive_swap_page(page);
+	} else {
+		migration_remove_reference(page);
+	}
 
 	mm->rss++;
 	pte = mk_pte(page, vma->vm_page_prot);
mm/mmigrate.c +166 -4

205 lines not on this page · raw message

mm/page_io.c +0 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/page_io.c linux-2.6.9-rc2-mm4.build/mm/page_io.c
@@ -15,7 +15,6 @@
 #include <linux/pagemap.h>
 #include <linux/swap.h>
 #include <linux/bio.h>
-#include <linux/swapops.h>
 #include <linux/writeback.h>
 #include <asm/pgtable.h>
 
mm/rmap.c +30 -16

59 lines not on this page · raw message

mm/shmem.c +0 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/shmem.c linux-2.6.9-rc2-mm4.build/mm/shmem.c
@@ -42,7 +42,6 @@
 #include <linux/vfs.h>
 #include <linux/blkdev.h>
 #include <linux/security.h>
-#include <linux/swapops.h>
 #include <linux/mempolicy.h>
 #include <linux/namei.h>
 #include <linux/xattr.h>

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/swapfile.c linux-2.6.9-rc2-mm4.build/mm/swapfile.c

mm/vmscan.c +3 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/vmscan.c linux-2.6.9-rc2-mm4.build/mm/vmscan.c
@@ -38,8 +38,6 @@
 #include <asm/tlbflush.h>
 #include <asm/div64.h>
 
-#include <linux/swapops.h>
-
 /*
  * The list of shrinker callbacks used by to apply pressure to
  * ageable caches.
@@ -459,7 +457,9 @@ int shrink_list(struct list_head *page_l
 		}
 
 #ifdef CONFIG_SWAP
-		if (PageSwapCache(page)) {
+		// FIXME: allow relocation of migrate cache pages 
+		// into real swap pages for swapout.
+		if (PageSwapCache(page) && !PageMigration(page)) {
 			swp_entry_t swap = { .val = page->private };
 			__delete_from_swap_cache(page);
 			write_unlock_irq(&mapping->tree_lock);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#1 lore raw

Hi, Marcelo

include/linux/swapops.h line 15

Quoted 7 lines · Marcelo Tosatti

Hi,
#define SWP_TYPE_SHIFT(e) (sizeof(e.val) * 8 - MAX_SWAPFILES_SHIFT)

include/linux/swapops.h +3 -1
-#define SWP_OFFSET_MASK(e)	((1UL << SWP_TYPE_SHIFT(e)) - 1)
+#define SWP_OFFSET_MASK(e)	((1UL << (SWP_TYPE_SHIFT(e))) - 1)
+
+#define MIGRATION_TYPE  (MAX_SWAPFILES - 1)

At the first glance, I think MIGRATION_TYPE=0 is better. #define MIGRATION_TYPE (0)

In swapfile.c::sys_swapon()
This code determines new swap_type for commanded swapon().
=============
p = swap_info;
for (type = 0 ; type < nr_swapfiles ; type++,p++)

         if (!(p->flags & SWP_USED))
               break;

error = -EPERM;
==============

set nr_swapfiles=1, swap_info[0].flags = SWP_USED
at boot time seems good. or fix swapon().

Thanks.
Kame <[email protected]>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#2 lore raw
Quoted 18 lines · Hiroyuki KAMEZAWA

Hi, Marcelo

Review

Quoted 7 lines · Marcelo Tosatti

Hi,
#define SWP_TYPE_SHIFT(e) (sizeof(e.val) * 8 - MAX_SWAPFILES_SHIFT)

(quoted diff) +3 -1
-#define SWP_OFFSET_MASK(e)	((1UL << SWP_TYPE_SHIFT(e)) - 1)
+#define SWP_OFFSET_MASK(e)	((1UL << (SWP_TYPE_SHIFT(e))) - 1)
+
+#define MIGRATION_TYPE  (MAX_SWAPFILES - 1)

At the first glance, I think MIGRATION_TYPE=0 is better. #define MIGRATION_TYPE (0)

In swapfile.c::sys_swapon()
This code determines new swap_type for commanded swapon().
=============
p = swap_info;
for (type = 0 ; type < nr_swapfiles ; type++,p++)

Quoted 7 lines
         if (!(p->flags & SWP_USED))
               break;

error = -EPERM;
==============

set nr_swapfiles=1, swap_info[0].flags = SWP_USED
at boot time seems good. or fix swapon().

Hi Hiroyuki,

Indeed.

This should do it?

swapfile.c 2004-10-26 11:34:03.284555296 -0200 renamed from swapfile.c.orig 2004-10-26 11:33:56.734551048 -0200 +7 -0
@@ -1370,6 +1370,13 @@ asmlinkage long sys_swapon(const char __
                swap_list_unlock();
                goto out;
        }
+
+       /* MAX_SWAPFILES-1 is reserved for migration pages */
+       if (type > MAX_SWAPFILES-1) {
+               swap_list_unlock();
+               goto out;
+       }
+
        if (type >= nr_swapfiles)
                nr_swapfiles = type+1;
        INIT_LIST_HEAD(&p->extent_list);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#3 lore raw
Quoted 10 lines · Marcelo Tosatti

This should do it?

swapfile.c 2004-10-26 11:34:03.284555296 -0200 renamed from swapfile.c.orig 2004-10-26 11:33:56.734551048 -0200 +0 -0
@@ -1370,6 +1370,13 @@ asmlinkage long sys_swapon(const char __
                swap_list_unlock();
                goto out;
        }

swapfile.c 2004-10-26 11:34:03.284555296 -0200 asmlinkage long sys_swapon(const char __ · line 1382

Quoted 10 lines
swapfile.c 2004-10-26 11:34:03.284555296 -0200 renamed from swapfile.c.orig 2004-10-26 11:33:56.734551048 -0200 +7 -0
+
+       /* MAX_SWAPFILES-1 is reserved for migration pages */
+       if (type > MAX_SWAPFILES-1) {
+               swap_list_unlock();
+               goto out;
+       }
+
        if (type >= nr_swapfiles)
                nr_swapfiles = type+1;
        INIT_LIST_HEAD(&p->extent_list);

This looks easier to read than my suggestion :).
But..
=========
if (type >= MIGRATION_TYPE) { /* MIGRATION_TYPE is set to maximum available swp_type. */

	goto out;
}

=========
Is maybe correct .

Thanks.

Kame <[email protected]>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#4 lore raw
In reply to Marcelo Tosatti

Hi,

I tested your patch and dead-locked has been occured in do_swap_page().

Quoted 17 lines

This is an improved version of the migration cache patch - thanks to everyone who contributed - Hirokazu, Iwamoto, Dave, Hugh.

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/memory.c linux-2.6.9-rc2-mm4.build/mm/memory.c

mm/memory.c +1 -1
@@ -1433,15 +1440,22 @@ again:
 		inc_page_state(pgmajfault);
 		grab_swap_token();
 	}
-
 	mark_page_accessed(page);
 	lock_page(page);
 	if (!PageSwapCache(page)) {
+		/* hiro: add !PageMigration(page) here */
 		/* page-migration has occured */

mm/memory.c again: · line 1457

Quoted 10 lines
mm/memory.c +6 -0
 		unlock_page(page);
 		page_cache_release(page);
 		goto again;
 	}
+	}
+
+
+	if (pte_is_migration(orig_pte)) {
+		mark_page_accessed(page);
+		lock_page(page);

The previous code will cause deadlock, as the page is already locked.

Quoted 9 lines
mm/memory.c +1 -1
+	}
 
 	/*
 	 * Back out if somebody else faulted in this pte while we
@@ -1459,10 +1473,14 @@ again:
 	}
 
 	/* The page isn't present yet, go ahead with the fault. */
-		

mm/memory.c again: · line 1482

Quoted 10 lines
mm/memory.c +7 -3
-	swap_free(entry);
-	if (vm_swap_full())
-		remove_exclusive_swap_page(page);
+
+	if (!pte_is_migration(orig_pte)) {
+		swap_free(entry);
+		if (vm_swap_full())
+			remove_exclusive_swap_page(page);
+	} else {
+		migration_remove_reference(page);

migration_remove_reference() also tries to lock the page that is already locked.

Quoted 18 lines
mm/memory.c +1 -0
+	}
 
 	mm->rss++;
 	pte = mk_pte(page, vma->vm_page_prot);

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/mmigrate.c linux-2.6.9-rc2-mm4.build/mm/mmigrate.c

mm/mmigrate.c +10 -0
 
+int migration_remove_reference(struct page *page)
+{
+	struct counter *c;
+	swp_entry_t entry;
+
+	entry.val = page->private;
+
+	read_lock_irq(&migration_space.tree_lock);
+
+	c = idr_find(&migration_idr, swp_offset(entry));

mm/mmigrate.c line 139

Quoted 10 lines
mm/mmigrate.c +10 -0
+
+	read_unlock_irq(&migration_space.tree_lock);
+
+	if (!c->i)
+		BUG();
+
+	c->i--;
+
+	if (!c->i) {
+		lock_page(page);

It will be dead-locked when this function is called from do_swap_page().

mm/mmigrate.c line 145

Quoted 6 lines
mm/mmigrate.c +6 -0
+		remove_from_migration_cache(page, page->private);
+		unlock_page(page);
+		kfree(c);
+	}
+		
+}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#5 lore raw
Quoted 4 lines · Hirokazu Takahashi

Hi,

I tested your patch and dead-locked has been occured in do_swap_page().

Hi,

Quoted 22 lines
Quoted 22 lines

This is an improved version of the migration cache patch - thanks to everyone who contributed - Hirokazu, Iwamoto, Dave, Hugh.

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/memory.c linux-2.6.9-rc2-mm4.build/mm/memory.c

mm/memory.c +2 -1
@@ -1433,15 +1440,22 @@ again:
 		inc_page_state(pgmajfault);
 		grab_swap_token();
 	}
-
 	mark_page_accessed(page);
 	lock_page(page);
 	if (!PageSwapCache(page)) {
+		/* hiro: add !PageMigration(page) here */
 		/* page-migration has occured */
 		unlock_page(page);
 		page_cache_release(page);
 		goto again;
 	}
+	}
Quoted 8 lines

mm/memory.c again: · line 1457

Quoted 5 lines
mm/memory.c +5 -0
+
+
+	if (pte_is_migration(orig_pte)) {
+		mark_page_accessed(page);
+		lock_page(page);

The previous code will cause deadlock, as the page is already locked.

Actually this one is fine - the page is not locked (its locked by the SwapCache pte path - not migration path)

if (pte_is_migration(pte))
lookup_migration_cache
else
old lookup swap cache
lock_page

if (pte_is_migration(pte))
mark_page_accessed
lock_page

Quoted 7 lines
Quoted 7 lines
mm/memory.c +1 -0
+	}
 
 	/*
 	 * Back out if somebody else faulted in this pte while we
@@ -1459,10 +1473,14 @@ again:
 	}
 
Quoted 15 lines

mm/memory.c again: · line 1482

Quoted 12 lines
mm/memory.c +7 -4
 	/* The page isn't present yet, go ahead with the fault. */
-		
-	swap_free(entry);
-	if (vm_swap_full())
-		remove_exclusive_swap_page(page);
+
+	if (!pte_is_migration(orig_pte)) {
+		swap_free(entry);
+		if (vm_swap_full())
+			remove_exclusive_swap_page(page);
+	} else {
+		migration_remove_reference(page);

migration_remove_reference() also tries to lock the page that is already locked.

Oh now thats a mess I've done.

I moved the locking into migration_remove_reference(), but didnt update the swap path code - I've moved it out again.

Quoted 22 lines
Quoted 22 lines
mm/memory.c +1 -0
+	}
 
 	mm->rss++;
 	pte = mk_pte(page, vma->vm_page_prot);

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/mmigrate.c linux-2.6.9-rc2-mm4.build/mm/mmigrate.c

mm/mmigrate.c +14 -0
 
+int migration_remove_reference(struct page *page)
+{
+	struct counter *c;
+	swp_entry_t entry;
+
+	entry.val = page->private;
+
+	read_lock_irq(&migration_space.tree_lock);
+
+	c = idr_find(&migration_idr, swp_offset(entry));
+
+	read_unlock_irq(&migration_space.tree_lock);
+
+	if (!c->i)
Quoted 8 lines

mm/mmigrate.c

Quoted 6 lines
mm/mmigrate.c +6 -0
+		BUG();
+
+	c->i--;
+
+	if (!c->i) {
+		lock_page(page);

It will be dead-locked when this function is called from do_swap_page().

Can you please try the tests with the following updated patch

Works for me

include/linux/mm.h +22 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/mm.h linux-2.6.9-rc2-mm4.build/include/linux/mm.h
@@ -251,6 +251,24 @@ extern int capture_page_range(unsigned l
  * files which need it (119 of them)
  */
 #include <linux/page-flags.h>
+#include <linux/swap.h>
+#include <linux/swapops.h> 
+
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 
 /*
  * Methods to modify the page usage count.
@@ -458,11 +476,14 @@ void page_address_init(void);
 #define PAGE_MAPPING_ANON	1
 
 extern struct address_space swapper_space;
+extern struct address_space migration_space;
 static inline struct address_space *page_mapping(struct page *page)
 {
 	struct address_space *mapping = page->mapping;
 
-	if (unlikely(PageSwapCache(page)))
+	if (unlikely(PageMigration(page)))
+		mapping = &migration_space;
+	else if (unlikely(PageSwapCache(page)))
 		mapping = &swapper_space;
 	else if (unlikely((unsigned long)mapping & PAGE_MAPPING_ANON))
 		mapping = NULL;
include/linux/swap.h +16 -0
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/swap.h linux-2.6.9-rc2-mm4.build/include/linux/swap.h
@@ -253,6 +253,7 @@ extern sector_t map_swap_page(struct swa
 extern struct swap_info_struct *get_swap_info_struct(unsigned);
 extern int can_share_swap_page(struct page *);
 extern int remove_exclusive_swap_page(struct page *);
+extern int migration_remove_entry(swp_entry_t);
 struct backing_dev_info;
 
 extern struct swap_list_t swap_list;
@@ -321,6 +322,21 @@ static inline swp_entry_t get_swap_page(
 #define grab_swap_token()  do { } while(0)
 #define has_swap_token(x) 0
 
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 #endif /* CONFIG_SWAP */
 #endif /* __KERNEL__*/
 #endif /* _LINUX_SWAP_H */
include/linux/swapops.h +25 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/swapops.h linux-2.6.9-rc2-mm4.build/include/linux/swapops.h
@@ -10,7 +10,9 @@
  * swp_entry_t's are *never* stored anywhere in their arch-dependent format.
  */
 #define SWP_TYPE_SHIFT(e)	(sizeof(e.val) * 8 - MAX_SWAPFILES_SHIFT)
-#define SWP_OFFSET_MASK(e)	((1UL << SWP_TYPE_SHIFT(e)) - 1)
+#define SWP_OFFSET_MASK(e)	((1UL << (SWP_TYPE_SHIFT(e))) - 1)
+
+#define MIGRATION_TYPE  (MAX_SWAPFILES - 1)
 
 /*
  * Store a type+offset into a swp_entry_t in an arch-independent format
@@ -30,8 +32,7 @@ static inline swp_entry_t swp_entry(unsi
  */
 static inline unsigned swp_type(swp_entry_t entry)
 {
-	return (entry.val >> SWP_TYPE_SHIFT(entry)) &
-			((1 << MAX_SWAPFILES_SHIFT) - 1);
+	return ((entry.val >> SWP_TYPE_SHIFT(entry)));
 }
 
 /*
@@ -68,3 +69,24 @@ static inline pte_t swp_entry_to_pte(swp
 	BUG_ON(pte_file(__swp_entry_to_pte(arch_entry)));
 	return __swp_entry_to_pte(arch_entry);
 }
+
+static inline int pte_is_migration(pte_t pte)
+{
+	unsigned long swp_type;
+	swp_entry_t arch_entry;
+
+	arch_entry = __pte_to_swp_entry(pte);
+
+	swp_type = __swp_type(arch_entry);
+
+	return swp_type == MIGRATION_TYPE;
+}
+
+static inline pte_t migration_entry_to_pte(swp_entry_t entry)
+{
+	swp_entry_t arch_entry;
+	
+	arch_entry = __swp_entry(MIGRATION_TYPE, swp_offset(entry));
+	return __swp_entry_to_pte(arch_entry);
+}
+
mm/fremap.c +8 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/fremap.c linux-2.6.9-rc2-mm4.build/mm/fremap.c
@@ -11,7 +11,6 @@
 #include <linux/file.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
-#include <linux/swapops.h>
 #include <linux/rmap.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
@@ -43,8 +42,14 @@ static inline void zap_pte(struct mm_str
 			}
 		}
 	} else {
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) { 
+				migration_remove_entry(swp_entry);
+			} else {
+				free_swap_and_cache(swp_entry);
+			}
+		}
 		pte_clear(ptep);
 	}
 }
mm/memory.c +26 -8
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/memory.c linux-2.6.9-rc2-mm4.build/mm/memory.c
@@ -53,7 +53,6 @@
 #include <asm/tlbflush.h>
 #include <asm/pgtable.h>
 
-#include <linux/swapops.h>
 #include <linux/elf.h>
 
 #ifndef CONFIG_DISCONTIGMEM
@@ -456,8 +455,13 @@ static void zap_pte_range(struct mmu_gat
 		 */
 		if (unlikely(details))
 			continue;
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) {
+				migration_remove_entry(swp_entry);
+			} else
+				free_swap_and_cache(swp_entry);
+		}
 		pte_clear(ptep);
 	}
 	pte_unmap(ptep-1);
@@ -1408,6 +1412,9 @@ static int do_swap_page(struct mm_struct
 	pte_unmap(page_table);
 	spin_unlock(&mm->page_table_lock);
 again:
+	if (pte_is_migration(orig_pte)) {
+		page = lookup_migration_cache(entry.val);
+	} else {
 	page = lookup_swap_cache(entry);
 	if (!page) {
  		swapin_readahead(entry, address, vma);
@@ -1433,15 +1440,22 @@ again:
 		inc_page_state(pgmajfault);
 		grab_swap_token();
 	}
-
 	mark_page_accessed(page);
 	lock_page(page);
 	if (!PageSwapCache(page)) {
+		/* hiro: add !PageMigration(page) here */
 		/* page-migration has occured */
 		unlock_page(page);
 		page_cache_release(page);
 		goto again;
 	}
+	}
+
+
+	if (pte_is_migration(orig_pte)) {
+		mark_page_accessed(page);
+		lock_page(page);
+	}
 
 	/*
 	 * Back out if somebody else faulted in this pte while we
@@ -1459,10 +1473,14 @@ again:
 	}
 
 	/* The page isn't present yet, go ahead with the fault. */
-		
-	swap_free(entry);
-	if (vm_swap_full())
-		remove_exclusive_swap_page(page);
+
+	if (!pte_is_migration(orig_pte)) {
+		swap_free(entry);
+		if (vm_swap_full())
+			remove_exclusive_swap_page(page);
+	} else {
+		migration_remove_reference(page);
+	}
 
 	mm->rss++;
 	pte = mk_pte(page, vma->vm_page_prot);
mm/mmigrate.c +168 -4

207 lines not on this page · raw message

mm/page_io.c +0 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/page_io.c linux-2.6.9-rc2-mm4.build/mm/page_io.c
@@ -15,7 +15,6 @@
 #include <linux/pagemap.h>
 #include <linux/swap.h>
 #include <linux/bio.h>
-#include <linux/swapops.h>
 #include <linux/writeback.h>
 #include <asm/pgtable.h>
 
mm/rmap.c +30 -16

59 lines not on this page · raw message

mm/shmem.c +0 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/shmem.c linux-2.6.9-rc2-mm4.build/mm/shmem.c
@@ -42,7 +42,6 @@
 #include <linux/vfs.h>
 #include <linux/blkdev.h>
 #include <linux/security.h>
-#include <linux/swapops.h>
 #include <linux/mempolicy.h>
 #include <linux/namei.h>
 #include <linux/xattr.h>
mm/swapfile.c +0 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/swapfile.c linux-2.6.9-rc2-mm4.build/mm/swapfile.c
@@ -29,7 +29,6 @@
 
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
-#include <linux/swapops.h>
 
 spinlock_t swaplock = SPIN_LOCK_UNLOCKED;
 unsigned int nr_swapfiles;
mm/vmscan.c +3 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/vmscan.c linux-2.6.9-rc2-mm4.build/mm/vmscan.c
@@ -38,8 +38,6 @@
 #include <asm/tlbflush.h>
 #include <asm/div64.h>
 
-#include <linux/swapops.h>
-
 /*
  * The list of shrinker callbacks used by to apply pressure to
  * ageable caches.
@@ -459,7 +457,9 @@ int shrink_list(struct list_head *page_l
 		}
 
 #ifdef CONFIG_SWAP
-		if (PageSwapCache(page)) {
+		// FIXME: allow relocation of migrate cache pages 
+		// into real swap pages for swapout.
+		if (PageSwapCache(page) && !PageMigration(page)) {
 			swp_entry_t swap = { .val = page->private };
 			__delete_from_swap_cache(page);
 			write_unlock_irq(&mapping->tree_lock);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#6 lore raw

Hi,

Quoted 11 lines
Quoted 1 line

The previous code will cause deadlock, as the page is already locked.

Actually this one is fine - the page is not locked (its locked by the SwapCache pte path - not migration path)

if (pte_is_migration(pte))
lookup_migration_cache
else
old lookup swap cache
lock_page

Quoted 3 lines

if (pte_is_migration(pte))
mark_page_accessed
lock_page

Oh, I understand.

Quoted 3 lines

Can you please try the tests with the following updated patch

Works for me

It didn't work without one fix.

+void remove_from_migration_cache(struct page *page, int id) +{
+ write_lock_irq(&migration_space.tree_lock);
+ idr_remove(&migration_idr, id);
+ radix_tree_delete(&migration_space.page_tree, id);
+ ClearPageSwapCache(page);
+ page->private = NULL;
+ write_unlock_irq(&migration_space.tree_lock);
+}

+int migration_remove_reference(struct page *page) +{
+ struct counter *c;
+ swp_entry_t entry;
+
+ entry.val = page->private;
+
+ read_lock_irq(&migration_space.tree_lock);
+
+ c = idr_find(&migration_idr, swp_offset(entry));
+
+ read_unlock_irq(&migration_space.tree_lock);
+
+ if (!c->i)
+ BUG();
+
+ c->i--;
+
+ if (!c->i) {
+ remove_from_migration_cache(page, page->private);
+ kfree(c);

page_cache_release(page) should be invoked here, as the count for the migration cache must be decreased.
With this fix, your migration cache started to work very fine!

+ }
+
+}

The attached patch is what I ported your patch to the latest version and I fixed the bug.

include/linux/mm.h renamed from include/linux/mm.h~migration_cache_marcelo +22 -1
diff -puN include/linux/mm.h~migration_cache_marcelo include/linux/mm.h
@@ -250,6 +250,24 @@ struct page {
  * files which need it (119 of them)
  */
 #include <linux/page-flags.h>
+#include <linux/swap.h>
+#include <linux/swapops.h> 
+
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 
 /*
  * Methods to modify the page usage count.
@@ -457,11 +475,14 @@ void page_address_init(void);
 #define PAGE_MAPPING_ANON	1
 
 extern struct address_space swapper_space;
+extern struct address_space migration_space;
 static inline struct address_space *page_mapping(struct page *page)
 {
 	struct address_space *mapping = page->mapping;
 
-	if (unlikely(PageSwapCache(page)))
+	if (unlikely(PageMigration(page)))
+		mapping = &migration_space;
+	else if (unlikely(PageSwapCache(page)))
 		mapping = &swapper_space;
 	else if (unlikely((unsigned long)mapping & PAGE_MAPPING_ANON))
 		mapping = NULL;
include/linux/swap.h renamed from include/linux/swap.h~migration_cache_marcelo +16 -0
diff -puN include/linux/swap.h~migration_cache_marcelo include/linux/swap.h
@@ -257,6 +257,7 @@ static inline int remove_exclusive_swap_
 {
 	return __remove_exclusive_swap_page(p, 0);
 }
+extern int migration_remove_entry(swp_entry_t);
 struct backing_dev_info;
 
 extern struct swap_list_t swap_list;
@@ -330,6 +331,21 @@ static inline swp_entry_t get_swap_page(
 #define put_swap_token(x) do { } while(0)
 #define grab_swap_token()  do { } while(0)
 #define has_swap_token(x) 0
+
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
 
 #endif /* CONFIG_SWAP */
 #endif /* __KERNEL__*/
include/linux/swapops.h renamed from include/linux/swapops.h~migration_cache_marcelo +24 -1
diff -puN include/linux/swapops.h~migration_cache_marcelo include/linux/swapops.h
@@ -10,7 +10,9 @@
  * swp_entry_t's are *never* stored anywhere in their arch-dependent format.
  */
 #define SWP_TYPE_SHIFT(e)	(sizeof(e.val) * 8 - MAX_SWAPFILES_SHIFT)
-#define SWP_OFFSET_MASK(e)	((1UL << SWP_TYPE_SHIFT(e)) - 1)
+#define SWP_OFFSET_MASK(e)	((1UL << (SWP_TYPE_SHIFT(e))) - 1)
+
+#define MIGRATION_TYPE  (MAX_SWAPFILES - 1)
 
 /*
  * Store a type+offset into a swp_entry_t in an arch-independent format
@@ -67,3 +69,24 @@ static inline pte_t swp_entry_to_pte(swp
 	BUG_ON(pte_file(__swp_entry_to_pte(arch_entry)));
 	return __swp_entry_to_pte(arch_entry);
 }
+
+static inline int pte_is_migration(pte_t pte)
+{
+	unsigned long swp_type;
+	swp_entry_t arch_entry;
+
+	arch_entry = __pte_to_swp_entry(pte);
+
+	swp_type = __swp_type(arch_entry);
+
+	return swp_type == MIGRATION_TYPE;
+}
+
+static inline pte_t migration_entry_to_pte(swp_entry_t entry)
+{
+	swp_entry_t arch_entry;
+	
+	arch_entry = __swp_entry(MIGRATION_TYPE, swp_offset(entry));
+	return __swp_entry_to_pte(arch_entry);
+}
+
mm/fremap.c renamed from mm/fremap.c~migration_cache_marcelo +8 -3
diff -puN mm/fremap.c~migration_cache_marcelo mm/fremap.c
@@ -11,7 +11,6 @@
 #include <linux/file.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
-#include <linux/swapops.h>
 #include <linux/rmap.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
@@ -43,8 +42,14 @@ static inline void zap_pte(struct mm_str
 			}
 		}
 	} else {
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) { 
+				migration_remove_entry(swp_entry);
+			} else {
+				free_swap_and_cache(swp_entry);
+			}
+		}
 		pte_clear(ptep);
 	}
 }
mm/memory.c renamed from mm/memory.c~migration_cache_marcelo +26 -8
diff -puN mm/memory.c~migration_cache_marcelo mm/memory.c
@@ -53,7 +53,6 @@
 #include <asm/tlbflush.h>
 #include <asm/pgtable.h>
 
-#include <linux/swapops.h>
 #include <linux/elf.h>
 
 #ifndef CONFIG_DISCONTIGMEM
@@ -456,8 +455,13 @@ static void zap_pte_range(struct mmu_gat
 		 */
 		if (unlikely(details))
 			continue;
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) {
+				migration_remove_entry(swp_entry);
+			} else
+				free_swap_and_cache(swp_entry);
+		}
 		pte_clear(ptep);
 	}
 	pte_unmap(ptep-1);
@@ -1533,6 +1537,9 @@ static int do_swap_page(struct mm_struct
 	pte_unmap(page_table);
 	spin_unlock(&mm->page_table_lock);
 again:
+	if (pte_is_migration(orig_pte)) {
+		page = lookup_migration_cache(entry.val);
+	} else {
 	page = lookup_swap_cache(entry);
 	if (!page) {
  		swapin_readahead(entry, address, vma);
@@ -1558,15 +1565,22 @@ again:
 		inc_page_state(pgmajfault);
 		grab_swap_token();
 	}
-
 	mark_page_accessed(page);
 	lock_page(page);
 	if (!PageSwapCache(page)) {
+		/* hiro: add !PageMigration(page) here */
 		/* page-migration has occured */
 		unlock_page(page);
 		page_cache_release(page);
 		goto again;
 	}
+	}
+
+
+	if (pte_is_migration(orig_pte)) {
+		mark_page_accessed(page);
+		lock_page(page);
+	}
 
 	/*
 	 * Back out if somebody else faulted in this pte while we
@@ -1584,10 +1598,14 @@ again:
 	}
 
 	/* The page isn't present yet, go ahead with the fault. */
-		
-	swap_free(entry);
-	if (vm_swap_full())
-		remove_exclusive_swap_page(page);
+
+	if (!pte_is_migration(orig_pte)) {
+		swap_free(entry);
+		if (vm_swap_full())
+			remove_exclusive_swap_page(page);
+	} else {
+		migration_remove_reference(page);
+	}
 
 	mm->rss++;
 	pte = mk_pte(page, vma->vm_page_prot);
mm/mmigrate.c renamed from mm/mmigrate.c~migration_cache_marcelo +169 -14

230 lines not on this page · raw message

mm/page_io.c renamed from mm/page_io.c~migration_cache_marcelo +0 -1
diff -puN mm/page_io.c~migration_cache_marcelo mm/page_io.c
@@ -15,7 +15,6 @@
 #include <linux/pagemap.h>
 #include <linux/swap.h>
 #include <linux/bio.h>
-#include <linux/swapops.h>
 #include <linux/writeback.h>
 #include <asm/pgtable.h>
 
mm/rmap.c renamed from mm/rmap.c~migration_cache_marcelo +30 -16

59 lines not on this page · raw message

mm/shmem.c renamed from mm/shmem.c~migration_cache_marcelo +0 -1
diff -puN mm/shmem.c~migration_cache_marcelo mm/shmem.c
@@ -42,7 +42,6 @@
 #include <linux/vfs.h>
 #include <linux/blkdev.h>
 #include <linux/security.h>
-#include <linux/swapops.h>
 #include <linux/mempolicy.h>
 #include <linux/namei.h>
 #include <linux/xattr.h>
mm/swapfile.c renamed from mm/swapfile.c~migration_cache_marcelo +0 -1
diff -puN mm/swapfile.c~migration_cache_marcelo mm/swapfile.c
@@ -33,7 +33,6 @@
 
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
-#include <linux/swapops.h>
 
 spinlock_t swaplock = SPIN_LOCK_UNLOCKED;
 unsigned int nr_swapfiles;
mm/vmscan.c renamed from mm/vmscan.c~migration_cache_marcelo +3 -3
diff -puN mm/vmscan.c~migration_cache_marcelo mm/vmscan.c
@@ -39,8 +39,6 @@
 #include <asm/tlbflush.h>
 #include <asm/div64.h>
 
-#include <linux/swapops.h>
-
 /*
  * The list of shrinker callbacks used by to apply pressure to
  * ageable caches.
@@ -460,7 +458,9 @@ int shrink_list(struct list_head *page_l
 		}
 
 #ifdef CONFIG_SWAP
-		if (PageSwapCache(page)) {
+		// FIXME: allow relocation of migrate cache pages 
+		// into real swap pages for swapout.
+		if (PageSwapCache(page) && !PageMigration(page)) {
 			swp_entry_t swap = { .val = page->private };
 			__delete_from_swap_cache(page);
 			write_unlock_irq(&mapping->tree_lock);

_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#7 lore raw
Quoted 57 lines · Hirokazu Takahashi

Hi,

Quoted 11 lines
Quoted 1 line

The previous code will cause deadlock, as the page is already locked.

Actually this one is fine - the page is not locked (its locked by the SwapCache pte path - not migration path)

if (pte_is_migration(pte))
lookup_migration_cache
else
old lookup swap cache
lock_page

Quoted 3 lines

if (pte_is_migration(pte))
mark_page_accessed
lock_page

Oh, I understand.

Quoted 3 lines

Can you please try the tests with the following updated patch

Works for me

It didn't work without one fix.

mm/mmigrate.c renamed from mm/mmigrate.c~migration_cache_marcelo +30 -0
+void remove_from_migration_cache(struct page *page, int id)
+{
+	write_lock_irq(&migration_space.tree_lock);
+        idr_remove(&migration_idr, id);
+	radix_tree_delete(&migration_space.page_tree, id);
+	ClearPageSwapCache(page);
+	page->private = NULL;
+	write_unlock_irq(&migration_space.tree_lock);
+}
 
+int migration_remove_reference(struct page *page)
+{
+	struct counter *c;
+	swp_entry_t entry;
+
+	entry.val = page->private;
+
+	read_lock_irq(&migration_space.tree_lock);
+
+	c = idr_find(&migration_idr, swp_offset(entry));
+
+	read_unlock_irq(&migration_space.tree_lock);
+
+	if (!c->i)
+		BUG();
+
+	c->i--;
+
+	if (!c->i) {
+		remove_from_migration_cache(page, page->private);
+		kfree(c);
Quoted 3 lines

page_cache_release(page) should be invoked here, as the count for the migration cache must be decreased.
With this fix, your migration cache started to work very fine!

Oh yes, I removed that by accident.

Quoted 8 lines
include/linux/swapops.h renamed from include/linux/swapops.h~migration_cache_marcelo +3 -0
+	}
+		
+}

The attached patch is what I ported your patch to the latest version and I fixed the bug.

It seems a hunk from your own tree leaked into this patch?

See above

Quoted 13 lines
mm/mmigrate.c renamed from mm/mmigrate.c~migration_cache_marcelo +0 -5
@@ -367,11 +527,6 @@ generic_migrate_page(struct page *page, 
 
 	/* map the newpage where the old page have been mapped. */
 	touch_unmapped_address(&vlist);
-	if (PageSwapCache(newpage)) {
-		lock_page(newpage);
-		__remove_exclusive_swap_page(newpage, 1);
-		unlock_page(newpage);
-	}
 
 	page->mapping = NULL;
 	unlock_page(page);

mm/mmigrate.c out_busy: · line 541

Quoted 9 lines
mm/mmigrate.c renamed from mm/mmigrate.c~migration_cache_marcelo +0 -5
@@ -383,11 +538,6 @@ out_busy:
 	/* Roll back all operations. */
 	rewind_page(page, newpage);
 	touch_unmapped_address(&vlist);
-	if (PageSwapCache(page)) {
-		lock_page(page);
-		__remove_exclusive_swap_page(page, 1);
-		unlock_page(page);
-	}
 	return ret;

This two hunks?

OK fine I'll update the patch with all fixes to
the newer version of -mhp, and start working
on the nonblocking version of the migration
functions.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#8 lore raw
In reply to Hirokazu Takahashi

Hi, Marcelo,

Oops has occurred on on my box with the migration cache patch after the long run. I tested it with Iwamoto's zone hotplug emulation patch.

There may be some migration cache related bugs there. Some pages seem to remain in the migration cache after page migration.

include/linux/swapops.h static inline pte_t swp_entry_to_pte(swp · line 91

Quoted 11 lines
mm/mmigrate.c renamed from mm/mmigrate.c~migration_cache_marcelo +3 -0
+	if (!c->i) {
+		remove_from_migration_cache(page, page->private);
+		kfree(c);

page_cache_release(page) should be invoked here, as the count for the migration cache must be decreased.
With this fix, your migration cache started to work very fine!

include/linux/swapops.h renamed from include/linux/swapops.h~migration_cache_marcelo +3 -0
+	}
+		
+}

Please take a look at the attached logs.

Some pages might have been put in LRU, and they seems to have been chosen as target pages to migrate again. The pages are handled as swap-cache pages accidentally. And both of the swp_offset in the logs seem to be very big. This looks like some pages in the migration cache haven't been released yet.

"swap file entry f8000223" means:
- swp_type is 0x1f, which is MIGRATION_TYPE.
- swp_offset is 0x223.

"swap file entry f8005ff4" means:
- swp_type is 0x1f, which is MIGRATION_TYPE.
- swp_offset is 0x5ff4.

I have no idea why this has happened yet.

Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f8000223
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f8000224
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f8000225
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f8000226
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f8000227
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f8000228
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f8000229
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f800022a
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f800022b
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f800022c
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f800022d
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f800022e
Oct 26 21:28:06 target1 kernel: swap_free: Bad swap file entry f800022f
Oct 26 21:28:08 target1 kernel: swap_free: Bad swap file entry f8000230
Oct 26 21:28:08 target1 kernel: swap_free: Bad swap file entry f8000231
Oct 26 21:28:08 target1 kernel: swap_free: Bad swap file entry f8000232
Oct 26 21:28:08 target1 kernel: swap_free: Bad swap file entry f8000233
Oct 26 21:28:08 target1 kernel: swap_free: Bad swap file entry f8000234
Oct 26 21:28:08 target1 kernel: swap_free: Bad swap file entry f8000235
Oct 26 21:28:08 target1 kernel: swap_free: Bad swap file entry f8000236
Oct 26 21:28:08 target1 kernel: Unable to handle kernel NULL pointer dereference at virtual address 00000028
Oct 26 21:28:08 target1 kernel:  printing eip:
Oct 26 21:28:08 target1 kernel: c028ef60
Oct 26 21:28:08 target1 kernel: *pde = 00000000
Oct 26 21:28:08 target1 kernel: Oops: 0002 [#1]
Oct 26 21:28:08 target1 kernel: SMP 
Oct 26 21:28:08 target1 kernel: Modules linked in:
Oct 26 21:28:08 target1 kernel: CPU:    0
Oct 26 21:28:08 target1 kernel: EIP:    0060:[_spin_lock+0/16]    Not tainted VLI
Oct 26 21:28:08 target1 kernel: EIP:    0060:[<c028ef60>]    Not tainted VLI
Oct 26 21:28:08 target1 kernel: EFLAGS: 00010282   (2.6.9-rc4-mm1) 
Oct 26 21:28:08 target1 kernel: EIP is at _spin_lock+0x0/0x10
Oct 26 21:28:09 target1 kernel: eax: 00000028   ebx: 00008097   ecx: ce0e7ed0   edx: ce0e7f24
Oct 26 21:28:09 target1 kernel: esi: 00000000   edi: f71fa8a0   ebp: 00000000   esp: ce0e7ec4
Oct 26 21:28:09 target1 kernel: ds: 007b   es: 007b   ss: 0068
Oct 26 21:28:09 target1 kernel: Process migrate131072 (pid: 1700, threadinfo=ce0e6000 task=cf01a100)
Oct 26 21:28:09 target1 kernel: Stack: c014ca72 00000000 00000000 00000001 cc9c7414 00000022 00000246 c013a5f1 
Oct 26 21:28:09 target1 kernel:        c1187420 f71fa8a0 c1187420 c03036e0 00000000 f71fa8a0 ce0e7f24 f71fa8a0 
Oct 26 21:28:09 target1 kernel:        00000000 c014ccc7 f71fa8a0 ce0e7f24 ce0e7f24 c0154b1f f71fa8a0 ce0e7f24 
Oct 26 21:28:09 target1 kernel: Call Trace:
Oct 26 21:28:09 target1 kernel:  [try_to_unmap_file+50/576] try_to_unmap_file+0x32/0x240
Oct 26 21:28:09 target1 kernel:  [<c014ca72>] try_to_unmap_file+0x32/0x240
Oct 26 21:28:09 target1 kernel:  [buffered_rmqueue+433/480] buffered_rmqueue+0x1b1/0x1e0
Oct 26 21:28:09 target1 kernel:  [<c013a5f1>] buffered_rmqueue+0x1b1/0x1e0
Oct 26 21:28:09 target1 kernel:  [try_to_unmap+71/89] try_to_unmap+0x47/0x59
Oct 26 21:28:09 target1 kernel:  [<c014ccc7>] try_to_unmap+0x47/0x59
Oct 26 21:28:09 target1 kernel:  [generic_migrate_page+95/688] generic_migrate_page+0x5f/0x2b0
Oct 26 21:28:09 target1 kernel:  [<c0154b1f>] generic_migrate_page+0x5f/0x2b0
Oct 26 21:28:09 target1 kernel:  [migrate_onepage+308/416] migrate_onepage+0x134/0x1a0
Oct 26 21:28:09 target1 kernel:  [<c0154ea4>] migrate_onepage+0x134/0x1a0
Oct 26 21:28:09 target1 kernel:  [migrate_page_common+0/256] migrate_page_common+0x0/0x100
Oct 26 21:28:09 target1 kernel:  [<c01545e0>] migrate_page_common+0x0/0x100
Oct 26 21:28:09 target1 kernel:  [try_to_migrate_pages+1144/1616] try_to_migrate_pages+0x478/0x650
Oct 26 21:28:09 target1 kernel:  [<c0155388>] try_to_migrate_pages+0x478/0x650
Oct 26 21:28:09 target1 kernel:  [mmigrated+146/201] mmigrated+0x92/0xc9
Oct 26 21:28:09 target1 kernel:  [<c01559b2>] mmigrated+0x92/0xc9
Oct 26 21:28:09 target1 kernel:  [mmigrated+0/201] mmigrated+0x0/0xc9
Oct 26 21:28:09 target1 kernel:  [<c0155920>] mmigrated+0x0/0xc9
Oct 26 21:28:09 target1 kernel:  [kernel_thread_helper+5/24] kernel_thread_helper+0x5/0x18
Oct 26 21:28:09 target1 kernel:  [<c010408d>] kernel_thread_helper+0x5/0x18
Oct 26 21:28:09 target1 kernel: Code: 00 00 01 74 05 e8 79 ec ff ff c3 ba 00 e0 ff ff 21 e2 81 42 14 00 01 00 00 f0 81 28 00 00 00 01 74 05 e8 5c ec ff ff c3 8d 76 00 <f0> fe 08 79 09 f3 90 80 38 00 7e f9 eb f2 c3 90 f0 81 28 00 00 
Oct 27 05:50:48 target1 kernel: swap_dup: Bad swap file entry f8005ff4
Oct 27 05:50:49 target1 kernel: ------------[ cut here ]------------
Oct 27 05:50:49 target1 kernel: kernel BUG at mm/mmigrate.c:115!
Oct 27 05:50:49 target1 kernel: invalid operand: 0000 [#1]
Oct 27 05:50:49 target1 kernel: SMP 
Oct 27 05:50:49 target1 kernel: Modules linked in:
Oct 27 05:50:49 target1 kernel: CPU:    0
Oct 27 05:50:49 target1 kernel: EIP:    0060:[migration_remove_entry+24/80]    Not tainted VLI
Oct 27 05:50:49 target1 kernel: EIP:    0060:[<c0154158>]    Not tainted VLI
Oct 27 05:50:49 target1 kernel: EFLAGS: 00010246   (2.6.9-rc4-mm1) 
Oct 27 05:50:49 target1 kernel: EIP is at migration_remove_entry+0x18/0x50
Oct 27 05:50:49 target1 kernel: eax: c0303700   ebx: 00000000   ecx: c0303704   edx: f8005ff4
Oct 27 05:50:49 target1 kernel: esi: 00000000   edi: 00000000   ebp: c12033a0   esp: cb681e08
Oct 27 05:50:49 target1 kernel: ds: 007b   es: 007b   ss: 0068
Oct 27 05:50:49 target1 kernel: Process migrate65536 (pid: 22493, threadinfo=cb680000 task=cee90a80)
Oct 27 05:50:49 target1 kernel: Stack: 00005ff4 c0145c61 f8005ff4 c68e9f64 00004000 bffd5000 c0000000 c40b5c00 
Oct 27 05:50:49 target1 kernel:        00000000 c0145ce5 c12033a0 c40b5bfc bffd5000 0002b000 00000000 bffd5000 
Oct 27 05:50:49 target1 kernel:        c40b5c00 c0000000 00000000 c0145d55 c12033a0 c40b5bfc bffd5000 0002b000 
Oct 27 05:50:49 target1 kernel: Call Trace:
Oct 27 05:50:49 target1 kernel:  [zap_pte_range+705/768] zap_pte_range+0x2c1/0x300
Oct 27 05:50:49 target1 kernel:  [<c0145c61>] zap_pte_range+0x2c1/0x300
Oct 27 05:50:49 target1 kernel:  [zap_pmd_range+69/112] zap_pmd_range+0x45/0x70
Oct 27 05:50:49 target1 kernel:  [<c0145ce5>] zap_pmd_range+0x45/0x70
Oct 27 05:50:49 target1 kernel:  [unmap_page_range+69/112] unmap_page_range+0x45/0x70
Oct 27 05:50:49 target1 kernel:  [<c0145d55>] unmap_page_range+0x45/0x70
Oct 27 05:50:49 target1 kernel:  [unmap_vmas+376/640] unmap_vmas+0x178/0x280
Oct 27 05:50:49 target1 kernel:  [<c0145ef8>] unmap_vmas+0x178/0x280
Oct 27 05:50:49 target1 kernel:  [exit_mmap+123/336] exit_mmap+0x7b/0x150
Oct 27 05:50:49 target1 kernel:  [<c014a64b>] exit_mmap+0x7b/0x150
Oct 27 05:50:49 target1 kernel:  [mmput+33/160] mmput+0x21/0xa0
Oct 27 05:50:49 target1 kernel:  [<c011a611>] mmput+0x21/0xa0
Oct 27 05:50:49 target1 kernel:  [touch_unmapped_address+208/256] touch_unmapped_address+0xd0/0x100
Oct 27 05:50:50 target1 kernel:  [<c014c480>] touch_unmapped_address+0xd0/0x100
Oct 27 05:50:50 target1 kernel:  [generic_migrate_page+482/688] generic_migrate_page+0x1e2/0x2b0
Oct 27 05:50:50 target1 kernel:  [<c0154cc2>] generic_migrate_page+0x1e2/0x2b0
Oct 27 05:50:50 target1 kernel:  [migrate_onepage+308/416] migrate_onepage+0x134/0x1a0
Oct 27 05:50:50 target1 kernel:  [<c0154ec4>] migrate_onepage+0x134/0x1a0
Oct 27 05:50:50 target1 kernel:  [migrate_page_common+0/256] migrate_page_common+0x0/0x100
Oct 27 05:50:50 target1 kernel:  [<c0154600>] migrate_page_common+0x0/0x100
Oct 27 05:50:50 target1 kernel:  [try_to_migrate_pages+648/1616] try_to_migrate_pages+0x288/0x650
Oct 27 05:50:50 target1 kernel:  [<c01551b8>] try_to_migrate_pages+0x288/0x650
Oct 27 05:50:50 target1 kernel:  [mmigrated+146/201] mmigrated+0x92/0xc9
Oct 27 05:50:50 target1 kernel:  [<c01559d2>] mmigrated+0x92/0xc9
Oct 27 05:50:50 target1 kernel:  [mmigrated+0/201] mmigrated+0x0/0xc9
Oct 27 05:50:50 target1 kernel:  [<c0155940>] mmigrated+0x0/0xc9
Oct 27 05:50:50 target1 kernel:  [kernel_thread_helper+5/24] kernel_thread_helper+0x5/0x18
Oct 27 05:50:50 target1 kernel:  [<c010408d>] kernel_thread_helper+0x5/0x18
Oct 27 05:50:50 target1 kernel: Code: 0c 00 00 00 00 ff 0d 80 36 30 c0 e8 03 af 13 00 5b 5e c3 53 8b 4c 24 08 51 68 00 37 30 c0 e8 40 23 fe ff 89 c3 58 85 db 5a 75 08 <0f> 0b 73 00 ee 43 2a c0 31 c0 f0 0f ab 03 19 c0 85 c0 74 07 89 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#9 lore raw
In reply to Marcelo Tosatti

Hi, Marcelo,

Quoted 5 lines

Hi,

This is an improved version of the migration cache patch - thanks to everyone who contributed - Hirokazu, Iwamoto, Dave, Hugh.

Some comments.

Quoted 14 lines

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/memory.c linux-2.6.9-rc2-mm4.build/mm/memory.c

mm/memory.c +3 -0
@@ -1408,6 +1412,9 @@ static int do_swap_page(struct mm_struct
 	pte_unmap(page_table);
 	spin_unlock(&mm->page_table_lock);
 again:
+	if (pte_is_migration(orig_pte)) {
+		page = lookup_migration_cache(entry.val);
+	} else {
 	page = lookup_swap_cache(entry);
 	if (!page) {
  		swapin_readahead(entry, address, vma);

mm/memory.c again: · line 1447

Quoted 9 lines
mm/memory.c +1 -1
@@ -1433,15 +1440,22 @@ again:
 		inc_page_state(pgmajfault);
 		grab_swap_token();
 	}
-
 	mark_page_accessed(page);
 	lock_page(page);
 	if (!PageSwapCache(page)) {
+		/* hiro: add !PageMigration(page) here */
 		/* page-migration has occured */

Now, !PageSwapCache(page) means the page isn't neither in the swap-cache nor in the migration-cache. The original code is enough.

Quoted 28 lines
mm/memory.c +7 -0
 		unlock_page(page);
 		page_cache_release(page);
 		goto again;
 	}
+	}
+
+
+	if (pte_is_migration(orig_pte)) {
+		mark_page_accessed(page);
+		lock_page(page);
+	}
 
 	/*
 	 * Back out if somebody else faulted in this pte while we

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/vmscan.c linux-2.6.9-rc2-mm4.build/mm/vmscan.c

mm/vmscan.c +0 -2
@@ -38,8 +38,6 @@
 #include <asm/tlbflush.h>
 #include <asm/div64.h>
 
-#include <linux/swapops.h>
-
 /*
  * The list of shrinker callbacks used by to apply pressure to
  * ageable caches.

mm/vmscan.c int shrink_list(struct list_head *page_l · line 461

Quoted 6 lines
mm/vmscan.c +2 -1
@@ -459,7 +457,9 @@ int shrink_list(struct list_head *page_l
 		}
 
 #ifdef CONFIG_SWAP
-		if (PageSwapCache(page)) {
+		// FIXME: allow relocation of migrate cache pages 
+		// into real swap pages for swapout.

In my thought, it would be better to remove a target page from the LRU lists prior to migration. So that it makes the swap code not to grab the page, which is in the migration cache.

mm/vmscan.c int shrink_list(struct list_head *page_l · line 465

Quoted 5 lines
mm/vmscan.c +1 -0
+		if (PageSwapCache(page) && !PageMigration(page)) {
 			swp_entry_t swap = { .val = page->private };
 			__delete_from_swap_cache(page);
 			write_unlock_irq(&mapping->tree_lock);

Thanks,
Hirokazu Takahashi.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#10 lore raw
Quoted 28 lines · Hirokazu Takahashi

Hi, Marcelo,

Quoted 5 lines

Hi,

This is an improved version of the migration cache patch - thanks to everyone who contributed - Hirokazu, Iwamoto, Dave, Hugh.

Some comments.

Quoted 18 lines

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/memory.c linux-2.6.9-rc2-mm4.build/mm/memory.c

mm/memory.c +3 -1
@@ -1408,6 +1412,9 @@ static int do_swap_page(struct mm_struct
 	pte_unmap(page_table);
 	spin_unlock(&mm->page_table_lock);
 again:
+	if (pte_is_migration(orig_pte)) {
+		page = lookup_migration_cache(entry.val);
+	} else {
 	page = lookup_swap_cache(entry);
 	if (!page) {
  		swapin_readahead(entry, address, vma);
@@ -1433,15 +1440,22 @@ again:
 		inc_page_state(pgmajfault);
 		grab_swap_token();
 	}
-
Quoted 8 lines

mm/memory.c again: · line 1447

Quoted 5 lines
mm/memory.c +1 -0
 	mark_page_accessed(page);
 	lock_page(page);
 	if (!PageSwapCache(page)) {
+		/* hiro: add !PageMigration(page) here */
 		/* page-migration has occured */

Now, !PageSwapCache(page) means the page isn't neither in the swap-cache nor in the migration-cache. The original code is enough.

OK!

Quoted 30 lines
Quoted 30 lines
mm/memory.c +7 -0
 		unlock_page(page);
 		page_cache_release(page);
 		goto again;
 	}
+	}
+
+
+	if (pte_is_migration(orig_pte)) {
+		mark_page_accessed(page);
+		lock_page(page);
+	}
 
 	/*
 	 * Back out if somebody else faulted in this pte while we

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/vmscan.c linux-2.6.9-rc2-mm4.build/mm/vmscan.c

mm/vmscan.c +0 -2
@@ -38,8 +38,6 @@
 #include <asm/tlbflush.h>
 #include <asm/div64.h>
 
-#include <linux/swapops.h>
-
 /*
  * The list of shrinker callbacks used by to apply pressure to
  * ageable caches.
@@ -459,7 +457,9 @@ int shrink_list(struct list_head *page_l
 		}
 
Quoted 9 lines

mm/vmscan.c int shrink_list(struct list_head *page_l · line 461

Quoted 4 lines
mm/vmscan.c +2 -1
 #ifdef CONFIG_SWAP
-		if (PageSwapCache(page)) {
+		// FIXME: allow relocation of migrate cache pages 
+		// into real swap pages for swapout.

In my thought, it would be better to remove a target page from the LRU lists prior to migration. So that it makes the swap code not to grab the page, which is in the migration cache.

I dont see a problem with having the pages on LRU - the reclaiming code sees it, but its unfreeable, so it doesnt touch it.

The reclaiming path should see its a migration page, unmap the pte's to it, remap them to swapcache pages (and ptes), so they can be swapped out on pressure.

Can you please expand your thoughts?

Quoted 5 lines

mm/vmscan.c int shrink_list(struct list_head *page_l · line 465

Quoted 5 lines
mm/vmscan.c +1 -0
+		if (PageSwapCache(page) && !PageMigration(page)) {
 			swp_entry_t swap = { .val = page->private };
 			__delete_from_swap_cache(page);
 			write_unlock_irq(&mapping->tree_lock);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#11 lore raw

Hi, Marcelo,

Quoted 16 lines
Quoted 6 lines
Quoted 6 lines

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/vmscan.c linux-2.6.9-rc2-mm4.build/mm/vmscan.c

mm/vmscan.c +0 -0
@@ -459,7 +457,9 @@ int shrink_list(struct list_head *page_l
 		}
 
Quoted 9 lines

mm/vmscan.c int shrink_list(struct list_head *page_l · line 461

Quoted 4 lines
mm/vmscan.c +2 -1
 #ifdef CONFIG_SWAP
-		if (PageSwapCache(page)) {
+		// FIXME: allow relocation of migrate cache pages 
+		// into real swap pages for swapout.

In my thought, it would be better to remove a target page from the LRU lists prior to migration. So that it makes the swap code not to grab the page, which is in the migration cache.

Quoted 8 lines

I dont see a problem with having the pages on LRU - the reclaiming code sees it, but its unfreeable, so it doesnt touch it.

The reclaiming path should see its a migration page, unmap the pte's to it, remap them to swapcache pages (and ptes), so they can be swapped out on pressure.

Can you please expand your thoughts?

I thought the easiest way to avoid the race condition was removing the page from LRU during memory migration. But there may be no problem about the page, which is unfreeable as you mentioned.

BTW, I wonder how the migration code avoid to choose some pages on LRU, which may have count == 0. This may happen the pages are going to be removed. We have to care about it.

Quoted 5 lines
Quoted 5 lines

mm/vmscan.c int shrink_list(struct list_head *page_l · line 465

Quoted 5 lines
mm/vmscan.c +1 -0
+		if (PageSwapCache(page) && !PageMigration(page)) {
 			swp_entry_t swap = { .val = page->private };
 			__delete_from_swap_cache(page);
 			write_unlock_irq(&mapping->tree_lock);

Thanks,
Hirokazu Takahashi.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#12 lore raw
Quoted 27 lines · Hirokazu Takahashi

Hi, Marcelo,

Quoted 16 lines
Quoted 6 lines
Quoted 6 lines

diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/vmscan.c linux-2.6.9-rc2-mm4.build/mm/vmscan.c

mm/vmscan.c +0 -0
@@ -459,7 +457,9 @@ int shrink_list(struct list_head *page_l
 		}
 
Quoted 9 lines

mm/vmscan.c int shrink_list(struct list_head *page_l · line 461

Quoted 4 lines
mm/vmscan.c +2 -1
 #ifdef CONFIG_SWAP
-		if (PageSwapCache(page)) {
+		// FIXME: allow relocation of migrate cache pages 
+		// into real swap pages for swapout.

In my thought, it would be better to remove a target page from the LRU lists prior to migration. So that it makes the swap code not to grab the page, which is in the migration cache.

Quoted 8 lines

I dont see a problem with having the pages on LRU - the reclaiming code sees it, but its unfreeable, so it doesnt touch it.

The reclaiming path should see its a migration page, unmap the pte's to it, remap them to swapcache pages (and ptes), so they can be swapped out on pressure.

Can you please expand your thoughts?

Quoted 8 lines

I thought the easiest way to avoid the race condition was removing the page from LRU during memory migration. But there may be no problem about the page, which is unfreeable as you mentioned.

BTW, I wonder how the migration code avoid to choose some pages on LRU, which may have count == 0. This may happen the pages are going to be removed. We have to care about it.

AFAICS its already done by __steal_page_from_lru(), which is used by grab_capturing_pages():

static int
grab_capturing_pages(struct list_head *page_list, unsigned long start_pfn,
unsigned long nr_pages)

{
        struct page *page;
        struct zone *zone;
        int rest = 0;
        int i;
                                                                                    
        for (i = 0; i < nr_pages; i++) {
                page = pfn_to_page(start_pfn + i);
                zone = page_zone(page);
                spin_lock_irq(&zone->lru_lock);
                if (page_under_capture(page)) {
                        if (PageLRU(page) && __steal_page_from_lru(zone, page))
                                list_add(&page->lru, page_list);
                        else
                                rest++;
                }
                spin_unlock_irq(&zone->lru_lock);
        }
        return rest;
}

Pages with reference count zero will be not be moved to the page list, and truncated pages seem to be handled nicely later on the migration codepath.

A quick search on Iwamoto's test utils shows no sign of truncate().

It would be nice to add more testcases (such as truncate() intensive application) to his testsuite.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#13 lore raw

At Tue, 26 Oct 2004 10:24:19 -0200,

Quoted 5 lines · Marcelo Tosatti

Pages with reference count zero will be not be moved to the page list, and truncated pages seem to be handled nicely later on the migration codepath.

A quick search on Iwamoto's test utils shows no sign of truncate().

IIRC, the easiest test method is file overwrite, such as

while true; do
tar zxvf ../some.tar.gz
done

Quoted 2 lines

It would be nice to add more testcases (such as truncate() intensive application) to his testsuite.

And it would be great to have an automated regression test suite. I wonder if OSDL's test harness(http://stp.sf.net/) could be used, but I had no chance to investigate any further.

--
IWAMOTO Toshihiro
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#14 lore raw
Quoted 16 lines · IWAMOTO Toshihiro

At Tue, 26 Oct 2004 10:24:19 -0200,

Quoted 5 lines · Marcelo Tosatti

Pages with reference count zero will be not be moved to the page list, and truncated pages seem to be handled nicely later on the migration codepath.

A quick search on Iwamoto's test utils shows no sign of truncate().

IIRC, the easiest test method is file overwrite, such as

while true; do
tar zxvf ../some.tar.gz
done

Quoted 6 lines
Quoted 2 lines

It would be nice to add more testcases (such as truncate() intensive application) to his testsuite.

And it would be great to have an automated regression test suite. I wonder if OSDL's test harness(http://stp.sf.net/) could be used, but I had no chance to investigate any further.

I dont think it is usable as it is because the benchmarks are fixed and you can't have scripts (your own commands) running as far as I remember - so its not possible to remove memory regions.

Other than that it should be fine - make a script to add/remove memory zones and let the benchmarks run.

Cliff, Judith, is that right?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#15 lore raw
In reply to Marcelo Tosatti

Hi,

Quoted 6 lines
Quoted 3 lines

BTW, I wonder how the migration code avoid to choose some pages on LRU, which may have count == 0. This may happen the pages are going to be removed. We have to care about it.

AFAICS its already done by __steal_page_from_lru(), which is used by grab_capturing_pages():

	:
Quoted 3 lines

Pages with reference count zero will be not be moved to the page list, and truncated pages seem to be handled nicely later on the migration codepath.

Ok, I see no problem about this with the current implementation.

BTW, now I'm just wondering migration_duplicate() should be called from copy_page_range(), since page-migration and fork() may work at the same time.

What do you think about this?

Thanks,
Hirokazu Takahashi.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#16 lore raw
Quoted 13 lines · Hirokazu Takahashi

Hi,

Quoted 6 lines
Quoted 3 lines

BTW, I wonder how the migration code avoid to choose some pages on LRU, which may have count == 0. This may happen the pages are going to be removed. We have to care about it.

AFAICS its already done by __steal_page_from_lru(), which is used by grab_capturing_pages():

	:
Quoted 3 lines

Pages with reference count zero will be not be moved to the page list, and truncated pages seem to be handled nicely later on the migration codepath.

Quoted 8 lines

Ok, I see no problem about this with the current implementation.

BTW, now I'm just wondering migration_duplicate() should be called from copy_page_range(), since page-migration and fork() may work at the same time.

What do you think about this?

Yep thats probably what caused your failures.

I'll prepare a new patch.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#17 lore raw
Quoted 21 lines · Marcelo Tosatti
Quoted 20 lines · Hirokazu Takahashi

Hi,

Quoted 6 lines
Quoted 3 lines

BTW, I wonder how the migration code avoid to choose some pages on LRU, which may have count == 0. This may happen the pages are going to be removed. We have to care about it.

AFAICS its already done by __steal_page_from_lru(), which is used by grab_capturing_pages():

	:
Quoted 3 lines

Pages with reference count zero will be not be moved to the page list, and truncated pages seem to be handled nicely later on the migration codepath.

Ok, I see no problem about this with the current implementation.

BTW, now I'm just wondering migration_duplicate() should be called from copy_page_range(), since page-migration and fork() may work at the same time.

Quoted 5 lines
Quoted 1 line

What do you think about this?

Yep thats probably what caused your failures.

I'll prepare a new patch.

Here it is - with the copy_page_range() fix as you pointed out, plus sys_swapon() fix as suggested by Hiroyuki.

I've also added a BUG() in case of swap_free() failure, so we get a backtrace.

Can you please test this - thanks.

include/linux/mm.h +22 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/mm.h linux-2.6.9-rc2-mm4.build/include/linux/mm.h
@@ -251,6 +251,24 @@ extern int capture_page_range(unsigned l
  * files which need it (119 of them)
  */
 #include <linux/page-flags.h>
+#include <linux/swap.h>
+#include <linux/swapops.h> 
+
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 
 /*
  * Methods to modify the page usage count.
@@ -458,11 +476,14 @@ void page_address_init(void);
 #define PAGE_MAPPING_ANON	1
 
 extern struct address_space swapper_space;
+extern struct address_space migration_space;
 static inline struct address_space *page_mapping(struct page *page)
 {
 	struct address_space *mapping = page->mapping;
 
-	if (unlikely(PageSwapCache(page)))
+	if (unlikely(PageMigration(page)))
+		mapping = &migration_space;
+	else if (unlikely(PageSwapCache(page)))
 		mapping = &swapper_space;
 	else if (unlikely((unsigned long)mapping & PAGE_MAPPING_ANON))
 		mapping = NULL;
include/linux/swap.h +16 -0
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/swap.h linux-2.6.9-rc2-mm4.build/include/linux/swap.h
@@ -253,6 +253,7 @@ extern sector_t map_swap_page(struct swa
 extern struct swap_info_struct *get_swap_info_struct(unsigned);
 extern int can_share_swap_page(struct page *);
 extern int remove_exclusive_swap_page(struct page *);
+extern int migration_remove_entry(swp_entry_t);
 struct backing_dev_info;
 
 extern struct swap_list_t swap_list;
@@ -321,6 +322,21 @@ static inline swp_entry_t get_swap_page(
 #define grab_swap_token()  do { } while(0)
 #define has_swap_token(x) 0
 
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 #endif /* CONFIG_SWAP */
 #endif /* __KERNEL__*/
 #endif /* _LINUX_SWAP_H */
include/linux/swapops.h +25 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/include/linux/swapops.h linux-2.6.9-rc2-mm4.build/include/linux/swapops.h
@@ -10,7 +10,9 @@
  * swp_entry_t's are *never* stored anywhere in their arch-dependent format.
  */
 #define SWP_TYPE_SHIFT(e)	(sizeof(e.val) * 8 - MAX_SWAPFILES_SHIFT)
-#define SWP_OFFSET_MASK(e)	((1UL << SWP_TYPE_SHIFT(e)) - 1)
+#define SWP_OFFSET_MASK(e)	((1UL << (SWP_TYPE_SHIFT(e))) - 1)
+
+#define MIGRATION_TYPE  (MAX_SWAPFILES - 1)
 
 /*
  * Store a type+offset into a swp_entry_t in an arch-independent format
@@ -30,8 +32,7 @@ static inline swp_entry_t swp_entry(unsi
  */
 static inline unsigned swp_type(swp_entry_t entry)
 {
-	return (entry.val >> SWP_TYPE_SHIFT(entry)) &
-			((1 << MAX_SWAPFILES_SHIFT) - 1);
+	return ((entry.val >> SWP_TYPE_SHIFT(entry)));
 }
 
 /*
@@ -68,3 +69,24 @@ static inline pte_t swp_entry_to_pte(swp
 	BUG_ON(pte_file(__swp_entry_to_pte(arch_entry)));
 	return __swp_entry_to_pte(arch_entry);
 }
+
+static inline int pte_is_migration(pte_t pte)
+{
+	unsigned long swp_type;
+	swp_entry_t arch_entry;
+
+	arch_entry = __pte_to_swp_entry(pte);
+
+	swp_type = __swp_type(arch_entry);
+
+	return swp_type == MIGRATION_TYPE;
+}
+
+static inline pte_t migration_entry_to_pte(swp_entry_t entry)
+{
+	swp_entry_t arch_entry;
+	
+	arch_entry = __swp_entry(MIGRATION_TYPE, swp_offset(entry));
+	return __swp_entry_to_pte(arch_entry);
+}
+
mm/fremap.c +8 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/fremap.c linux-2.6.9-rc2-mm4.build/mm/fremap.c
@@ -11,7 +11,6 @@
 #include <linux/file.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
-#include <linux/swapops.h>
 #include <linux/rmap.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
@@ -43,8 +42,14 @@ static inline void zap_pte(struct mm_str
 			}
 		}
 	} else {
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) { 
+				migration_remove_entry(swp_entry);
+			} else {
+				free_swap_and_cache(swp_entry);
+			}
+		}
 		pte_clear(ptep);
 	}
 }
mm/memory.c +33 -9

86 lines not on this page · raw message

mm/mmigrate.c +173 -6

235 lines not on this page · raw message

mm/page_io.c +0 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/page_io.c linux-2.6.9-rc2-mm4.build/mm/page_io.c
@@ -15,7 +15,6 @@
 #include <linux/pagemap.h>
 #include <linux/swap.h>
 #include <linux/bio.h>
-#include <linux/swapops.h>
 #include <linux/writeback.h>
 #include <asm/pgtable.h>
 
mm/rmap.c +30 -16

59 lines not on this page · raw message

mm/shmem.c +0 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/shmem.c linux-2.6.9-rc2-mm4.build/mm/shmem.c
@@ -42,7 +42,6 @@
 #include <linux/vfs.h>
 #include <linux/blkdev.h>
 #include <linux/security.h>
-#include <linux/swapops.h>
 #include <linux/mempolicy.h>
 #include <linux/namei.h>
 #include <linux/xattr.h>
mm/swapfile.c +8 -1
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/swapfile.c linux-2.6.9-rc2-mm4.build/mm/swapfile.c
@@ -29,7 +29,6 @@
 
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
-#include <linux/swapops.h>
 
 spinlock_t swaplock = SPIN_LOCK_UNLOCKED;
 unsigned int nr_swapfiles;
@@ -230,6 +229,7 @@ bad_device:
 	printk(KERN_ERR "swap_free: %s%08lx\n", Unused_file, entry.val);
 	goto out;
 bad_nofile:
+	BUG();
 	printk(KERN_ERR "swap_free: %s%08lx\n", Bad_file, entry.val);
 out:
 	return NULL;
@@ -1369,6 +1370,13 @@ asmlinkage long sys_swapon(const char __
 		swap_list_unlock();
 		goto out;
 	}
+
+	/* MIGRATION_TYPE is reserved for migration pages */
+	if (type >= MIGRATION_TYPE) {
+		swap_list_unlock();
+		goto out;
+	}
+
 	if (type >= nr_swapfiles)
 		nr_swapfiles = type+1;
 	INIT_LIST_HEAD(&p->extent_list);
mm/vmscan.c +3 -3
diff -Nur --show-c-function linux-2.6.9-rc2-mm4.mhp.orig/mm/vmscan.c linux-2.6.9-rc2-mm4.build/mm/vmscan.c
@@ -38,8 +38,6 @@
 #include <asm/tlbflush.h>
 #include <asm/div64.h>
 
-#include <linux/swapops.h>
-
 /*
  * The list of shrinker callbacks used by to apply pressure to
  * ageable caches.
@@ -459,7 +457,9 @@ int shrink_list(struct list_head *page_l
 		}
 
 #ifdef CONFIG_SWAP
-		if (PageSwapCache(page)) {
+		// FIXME: allow relocation of migrate cache pages 
+		// into real swap pages for swapout.
+		if (PageSwapCache(page) && !PageMigration(page)) {
 			swp_entry_t swap = { .val = page->private };
 			__delete_from_swap_cache(page);
 			write_unlock_irq(&mapping->tree_lock);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#18 lore raw
Quoted 4 lines · Marcelo Tosatti
include/linux/mm.h +4 -0
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+

include/linux/mm.h extern int capture_page_range(unsigned l · line 270

Quoted 10 lines
include/linux/mm.h +10 -0
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}

Don't we usually try to keep the Page*() operations to be strict page->flags checks? Should this be page_migration() or something similar instead? --
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#19 lore raw
Quoted 11 lines · Dave Hansen
Quoted 10 lines · Marcelo Tosatti
include/linux/mm.h +10 -0
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
Quoted 8 lines

include/linux/mm.h extern int capture_page_range(unsigned l · line 270

Quoted 4 lines
include/linux/mm.h +4 -0
+                return 0;
+
+        return 1;
+}

Don't we usually try to keep the Page*() operations to be strict page->flags checks? Should this be page_migration() or something similar instead?

Yeah I think page_migration() will be more conformant to the current macros.

Will do it, and upgrade to the latest -mhp. What is it again? --
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#20 lore raw
Quoted 4 lines · Marcelo Tosatti

Yeah I think page_migration() will be more conformant to the current macros.

Will do it, and upgrade to the latest -mhp. What is it again?

http://sprucegoose.sr71.net/patches/2.6.9-mm1-mhp1/
http://sprucegoose.sr71.net/patches/patch-2.6.9-mm1-mhp1.gz

I have one against 2.6.10-rc1-mm1, but it's not quite ready yet. --
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#21 lore raw
In reply to Marcelo Tosatti
Quoted 21 lines · Marcelo Tosatti
Quoted 11 lines · Dave Hansen
Quoted 10 lines · Marcelo Tosatti
include/linux/mm.h +10 -0
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
Quoted 8 lines

include/linux/mm.h extern int capture_page_range(unsigned l · line 270

Quoted 4 lines
include/linux/mm.h +4 -0
+                return 0;
+
+        return 1;
+}

Don't we usually try to keep the Page*() operations to be strict page->flags checks? Should this be page_migration() or something similar instead?

Quoted 4 lines

Yeah I think page_migration() will be more conformant to the current macros.

Will do it, and upgrade to the latest -mhp. What is it again?

Can't boot 2.6.9-mm1-mhp on my dual P4 - reverting -mhp makes it happy again (with same .config file). Freezes after "OK, now booting the kernel".

Will stick to -rc4-mm1 for now.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#22 lore raw

Hi,

Quoted 5 lines

Can't boot 2.6.9-mm1-mhp on my dual P4 - reverting -mhp makes it happy again (with same .config file). Freezes after "OK, now booting the kernel".

Will stick to -rc4-mm1 for now.

I guess the problem may be solved with the attached patch Kame-san made.

From: Hiroyuki KAMEZAWA <[email protected]>
To: Dave Hansen <[email protected]>
Cc: Hirokazu Takahashi <[email protected]>,
[email protected]
Date: Mon, 01 Nov 2004 19:23:15 +0900
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.3) Gecko/20040910

Quoted 10 lines · Dave Hansen
Quoted 2 lines · Hirokazu Takahashi

BTW, linux-2.6.9-mm1-mhp1 often causes startup freeze on my box. I don't know why.

It would be very helpful to diagnose it. Where does the freeze occur? Can you dump the console? Did you boot with "debug" on the kernel command line? Have you tried sysrq?

This is a patch for this i386 problem. alloc_memmap() is called twice and this hits new BUG_ON() :)

Thanks.
Kame <[email protected]>

---

  linux-2.6.9-mm1-mhp-kamezawa/arch/i386/kernel/setup.c |   13 -------------
  1 files changed, 13 deletions(-)
arch/i386/kernel/setup.c renamed from arch/i386/kernel/setup.c~cleanup +0 -13
diff -puN arch/i386/kernel/setup.c~cleanup arch/i386/kernel/setup.c
@@ -1393,19 +1393,6 @@ void __init setup_arch(char **cmdline_p)
  #endif
  	paging_init();
 
-	/*
-	 * NOTE: at this point the bootmem allocator is fully available.
-	 */
-
-#ifdef CONFIG_NONLINEAR
-	{
-		/* use alloc_node_mem_map() instead - daveh */
-		struct page *lmem_map;
-		lmem_map = alloc_bootmem(max_pfn * sizeof(struct page));
-		alloc_memmap(lmem_map, 0, max_pfn);
-	}
-#endif
-
  	zone_sizes_init();
 
  #ifdef CONFIG_EARLY_PRINTK

_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#23 lore raw
In reply to Marcelo Tosatti

Hi, Marcelo,

I happened to meet a bug.

Quoted 7 lines
Quoted 3 lines

Yep thats probably what caused your failures.

I'll prepare a new patch.

Here it is - with the copy_page_range() fix as you pointed out, plus sys_swapon() fix as suggested by Hiroyuki.

Quoted 6 lines

I've also added a BUG() in case of swap_free() failure, so we get a backtrace.

Can you please test this - thanks.

From the attached message, lookup_migration_cache() returned NULL

in do_swap_page(). There might be a race condition related the migration cache.

Nov  5 22:18:22 target1 kernel: Unable to handle kernel NULL pointer dereference at virtual address 00000000
Nov  5 22:18:22 target1 kernel:  printing eip:
Nov  5 22:18:22 target1 kernel: c0141364
Nov  5 22:18:22 target1 kernel: *pde = 00000000
Nov  5 22:18:22 target1 kernel: Oops: 0000 [#1]
Nov  5 22:18:22 target1 kernel: SMP 
Nov  5 22:18:22 target1 kernel: Modules linked in:
Nov  5 22:18:22 target1 kernel: CPU:    0
Nov  5 22:18:22 target1 kernel: EIP:    0060:[mark_page_accessed+4/80]    Not tainted VLI
Nov  5 22:18:22 target1 kernel: EIP:    0060:[<c0141364>]    Not tainted VLI
Nov  5 22:18:22 target1 kernel: EFLAGS: 00010246   (2.6.9-rc4-mm1) 
Nov  5 22:18:22 target1 kernel: EIP is at mark_page_accessed+0x4/0x50
Nov  5 22:18:22 target1 kernel: eax: 00000000   ebx: 00000000   ecx: c0304700   edx: f8000005
Nov  5 22:18:22 target1 kernel: esi: 00000000   edi: 0000053e   ebp: b72eafe0   esp: ce12be90
Nov  5 22:18:22 target1 kernel: ds: 007b   es: 007b   ss: 0068
Nov  5 22:18:22 target1 kernel: Process grep (pid: 2441, threadinfo=ce12a000 task=cf66d550)
Nov  5 22:18:22 target1 kernel: Stack: f8000005 00000000 c01474d4 c01065c2 00000001 ef720000 cf2655f4 ccc13b70 
Nov  5 22:18:22 target1 kernel:        b72eafe0 cf76cdc0 c0147cf4 cf76cdc0 cf2655f4 b72eafe0 c84f8ba8 ccc13b70 
Nov  5 22:18:22 target1 kernel:        0000053e 00000000 b72eafe0 cf76cdc0 cf2655f4 cf66d550 c0114835 cf76cdc0 
Nov  5 22:18:22 target1 kernel: Call Trace:
Nov  5 22:18:22 target1 kernel:  [do_swap_page+372/784] do_swap_page+0x174/0x310
Nov  5 22:18:22 target1 kernel:  [<c01474d4>] do_swap_page+0x174/0x310
Nov  5 22:18:22 target1 kernel:  [apic_timer_interrupt+26/32] apic_timer_interrupt+0x1a/0x20
Nov  5 22:18:22 target1 kernel:  [<c01065c2>] apic_timer_interrupt+0x1a/0x20
Nov  5 22:18:22 target1 kernel:  [handle_mm_fault+228/352] handle_mm_fault+0xe4/0x160
Nov  5 22:18:22 target1 kernel:  [<c0147cf4>] handle_mm_fault+0xe4/0x160
Nov  5 22:18:22 target1 kernel:  [do_page_fault+469/1487] do_page_fault+0x1d5/0x5cf
Nov  5 22:18:22 target1 kernel:  [<c0114835>] do_page_fault+0x1d5/0x5cf
Nov  5 22:18:22 target1 kernel:  [run_timer_softirq+481/496] run_timer_softirq+0x1e1/0x1f0
Nov  5 22:18:22 target1 kernel:  [<c0124821>] run_timer_softirq+0x1e1/0x1f0
Nov  5 22:18:22 target1 kernel:  [update_wall_time+21/64] update_wall_time+0x15/0x40
Nov  5 22:18:22 target1 kernel:  [<c01244d5>] update_wall_time+0x15/0x40
Nov  5 22:18:22 target1 kernel:  [do_timer+46/192] do_timer+0x2e/0xc0
Nov  5 22:18:22 target1 kernel:  [<c012486e>] do_timer+0x2e/0xc0
Nov  5 22:18:22 target1 kernel:  [timer_interrupt+72/240] timer_interrupt+0x48/0xf0
Nov  5 22:18:22 target1 kernel:  [<c010b148>] timer_interrupt+0x48/0xf0
Nov  5 22:18:22 target1 kernel:  [timer_interrupt+229/240] timer_interrupt+0xe5/0xf0
Nov  5 22:18:22 target1 kernel:  [<c010b1e5>] timer_interrupt+0xe5/0xf0
Nov  5 22:18:22 target1 kernel:  [handle_IRQ_event+44/96] handle_IRQ_event+0x2c/0x60
Nov  5 22:18:22 target1 kernel:  [<c013530c>] handle_IRQ_event+0x2c/0x60
Nov  5 22:18:22 target1 kernel:  [__do_IRQ+280/336] __do_IRQ+0x118/0x150
Nov  5 22:18:22 target1 kernel:  [<c0135458>] __do_IRQ+0x118/0x150
Nov  5 22:18:22 target1 kernel:  [__do_IRQ+318/336] __do_IRQ+0x13e/0x150
Nov  5 22:18:22 target1 kernel:  [<c013547e>] __do_IRQ+0x13e/0x150
Nov  5 22:18:22 target1 kernel:  [do_page_fault+0/1487] do_page_fault+0x0/0x5cf
Nov  5 22:18:22 target1 kernel:  [<c0114660>] do_page_fault+0x0/0x5cf
Nov  5 22:18:22 target1 kernel:  [error_code+45/56] error_code+0x2d/0x38
Nov  5 22:18:22 target1 kernel:  [<c010663d>] error_code+0x2d/0x38
Nov  5 22:18:22 target1 kernel: Code: 1c 85 20 80 3f c0 01 da ff 42 38 51 9d 8d 86 00 01 00 00 e8 ef e9 14 00 5b 5e 5f c3 8d 74 26 00 8d bc 27 00 00 00 00 56 53 89 c3 <8b> 03 83 e0 40 75 25 8b 03 be 02 00 00 00 83 e0 04 74 19 8b 03 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#24 lore raw
Quoted 11 lines · Hirokazu Takahashi

Hi, Marcelo,

I happened to meet a bug.

Quoted 7 lines
Quoted 3 lines

Yep thats probably what caused your failures.

I'll prepare a new patch.

Here it is - with the copy_page_range() fix as you pointed out, plus sys_swapon() fix as suggested by Hiroyuki.

Quoted 8 lines
Quoted 6 lines

I've also added a BUG() in case of swap_free() failure, so we get a backtrace.

Can you please test this - thanks.

From the attached message, lookup_migration_cache() returned NULL

in do_swap_page(). There might be a race condition related the migration cache.

Hi Hirokazu!

The problem is that another thread can fault in the pte (removing the radix tree entry) while the current thread dropped the page_table_lock - which explains the NULL lookup_migration_cache. The swap code handles this situation, but I've completly missed it.

Updated patch attached.

Extreme thanks for your testing, its being crucial!

We're getting there.

do_swap_page now does:

again:
+ if (pte_is_migration(orig_pte)) {
+ page = lookup_migration_cache(entry.val);
+ if (!page) {

+                       spin_lock(&mm->page_table_lock);
+                       page_table = pte_offset_map(pmd, address);
+                       if (likely(pte_same(*page_table, orig_pte)))
+                               ret = VM_FAULT_OOM;
+                       else
+                               ret = VM_FAULT_MINOR;
+                       pte_unmap(page_table);
+                       spin_unlock(&mm->page_table_lock);
+                       goto out;

+ }
+ } else {

include/linux/mm.h +22 -1
diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/include/linux/mm.h linux-2.6.9-rc2-mm4.build/include/linux/mm.h
@@ -251,6 +251,24 @@
  * files which need it (119 of them)
  */
 #include <linux/page-flags.h>
+#include <linux/swap.h>
+#include <linux/swapops.h> 
+
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 
 /*
  * Methods to modify the page usage count.
@@ -458,11 +476,14 @@
 #define PAGE_MAPPING_ANON	1
 
 extern struct address_space swapper_space;
+extern struct address_space migration_space;
 static inline struct address_space *page_mapping(struct page *page)
 {
 	struct address_space *mapping = page->mapping;
 
-	if (unlikely(PageSwapCache(page)))
+	if (unlikely(PageMigration(page)))
+		mapping = &migration_space;
+	else if (unlikely(PageSwapCache(page)))
 		mapping = &swapper_space;
 	else if (unlikely((unsigned long)mapping & PAGE_MAPPING_ANON))
 		mapping = NULL;
include/linux/swap.h +16 -0
diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/include/linux/swap.h linux-2.6.9-rc2-mm4.build/include/linux/swap.h
@@ -253,6 +253,7 @@
 extern struct swap_info_struct *get_swap_info_struct(unsigned);
 extern int can_share_swap_page(struct page *);
 extern int remove_exclusive_swap_page(struct page *);
+extern int migration_remove_entry(swp_entry_t);
 struct backing_dev_info;
 
 extern struct swap_list_t swap_list;
@@ -321,6 +322,21 @@
 #define grab_swap_token()  do { } while(0)
 #define has_swap_token(x) 0
 
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 #endif /* CONFIG_SWAP */
 #endif /* __KERNEL__*/
 #endif /* _LINUX_SWAP_H */
include/linux/swapops.h +24 -1
diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/include/linux/swapops.h linux-2.6.9-rc2-mm4.build/include/linux/swapops.h
@@ -10,7 +10,9 @@
  * swp_entry_t's are *never* stored anywhere in their arch-dependent format.
  */
 #define SWP_TYPE_SHIFT(e)	(sizeof(e.val) * 8 - MAX_SWAPFILES_SHIFT)
-#define SWP_OFFSET_MASK(e)	((1UL << SWP_TYPE_SHIFT(e)) - 1)
+#define SWP_OFFSET_MASK(e)	((1UL << (SWP_TYPE_SHIFT(e))) - 1)
+
+#define MIGRATION_TYPE  (MAX_SWAPFILES - 1)
 
 /*
  * Store a type+offset into a swp_entry_t in an arch-independent format
@@ -68,3 +69,24 @@
 	BUG_ON(pte_file(__swp_entry_to_pte(arch_entry)));
 	return __swp_entry_to_pte(arch_entry);
 }
+
+static inline int pte_is_migration(pte_t pte)
+{
+	unsigned long swp_type;
+	swp_entry_t arch_entry;
+
+	arch_entry = __pte_to_swp_entry(pte);
+
+	swp_type = __swp_type(arch_entry);
+
+	return swp_type == MIGRATION_TYPE;
+}
+
+static inline pte_t migration_entry_to_pte(swp_entry_t entry)
+{
+	swp_entry_t arch_entry;
+	
+	arch_entry = __swp_entry(MIGRATION_TYPE, swp_offset(entry));
+	return __swp_entry_to_pte(arch_entry);
+}
+
mm/fremap.c +8 -3
diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/mm/fremap.c linux-2.6.9-rc2-mm4.build/mm/fremap.c
@@ -11,7 +11,6 @@
 #include <linux/file.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
-#include <linux/swapops.h>
 #include <linux/rmap.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
@@ -43,8 +42,14 @@
 			}
 		}
 	} else {
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) { 
+				migration_remove_entry(swp_entry);
+			} else {
+				free_swap_and_cache(swp_entry);
+			}
+		}
 		pte_clear(ptep);
 	}
 }
mm/memory.c +44 -9

97 lines not on this page · raw message

mm/mmigrate.c +169 -5

212 lines not on this page · raw message

return 0;

mm/page_io.c +0 -1
diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/mm/page_io.c linux-2.6.9-rc2-mm4.build/mm/page_io.c
@@ -15,7 +15,6 @@
 #include <linux/pagemap.h>
 #include <linux/swap.h>
 #include <linux/bio.h>
-#include <linux/swapops.h>
 #include <linux/writeback.h>
 #include <asm/pgtable.h>
 
mm/rmap.c +30 -16

59 lines not on this page · raw message

mm/shmem.c +0 -1
diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/mm/shmem.c linux-2.6.9-rc2-mm4.build/mm/shmem.c
@@ -42,7 +42,6 @@
 #include <linux/vfs.h>
 #include <linux/blkdev.h>
 #include <linux/security.h>
-#include <linux/swapops.h>
 #include <linux/mempolicy.h>
 #include <linux/namei.h>
 #include <linux/xattr.h>
mm/swapfile.c +8 -1
diff -Nur linux-2.6.9-rc2-mm4.mhp.orig/mm/swapfile.c linux-2.6.9-rc2-mm4.build/mm/swapfile.c
@@ -29,7 +29,6 @@
 
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
-#include <linux/swapops.h>
 
 spinlock_t swaplock = SPIN_LOCK_UNLOCKED;
 unsigned int nr_swapfiles;
@@ -230,6 +229,7 @@
 	printk(KERN_ERR "swap_free: %s%08lx\n", Unused_file, entry.val);
 	goto out;
 bad_nofile:
+	BUG();
 	printk(KERN_ERR "swap_free: %s%08lx\n", Bad_file, entry.val);
 out:
 	return NULL;
@@ -1369,6 +1370,13 @@
 		swap_list_unlock();
 		goto out;
 	}
+
+	/* MIGRATION_TYPE is reserved for migration pages */
+	if (type >= MIGRATION_TYPE) {
+		swap_list_unlock();
+		goto out;
+	}
+
 	if (type >= nr_swapfiles)
 		nr_swapfiles = type+1;
 	INIT_LIST_HEAD(&p->extent_list);
#25 lore raw

Hi Marcelo,

I've been testing the memory migration code with your patch. I found problems and I think the attached patch would fix some of them.

One of the problems is a race condition between add_to_migration_cache() and try_to_unmap(). Some pages in the migration cache cannot be removed with the current implementation. Please suppose
a process space might be removed between them. In this case no one can remove pages the process had from the migration cache, because they can be removed only when the pagetables pointed the pages.

Therefore, I made pages removed from the migration cache at the end of generic_migrate_page() if they remain in the cache.

The another is a fork() related problem. If fork() has occurred during page migration, the previous work may not go well. pages may not be removed from the migration cache.

So I made the swapcode ignore pages in the migration cache. However, as you know this is just a workaround and not a correct way to fix it.

Quoted 21 lines

Hi Hirokazu!

The problem is that another thread can fault in the pte (removing the radix tree entry) while the current thread dropped the page_table_lock - which explains the NULL lookup_migration_cache. The swap code handles this situation, but I've completly missed it.

Updated patch attached.

Extreme thanks for your testing, its being crucial!

We're getting there.

do_swap_page now does:

again:

(quoted diff) +4 -0
+       if (pte_is_migration(orig_pte)) {
+               page = lookup_migration_cache(entry.val);
+               if (!page) {
+                       spin_lock(&mm->page_table_lock);

Review

Quoted 10 lines
(quoted diff) +10 -0
+                       page_table = pte_offset_map(pmd, address);
+                       if (likely(pte_same(*page_table, orig_pte)))
+                               ret = VM_FAULT_OOM;
+                       else
+                               ret = VM_FAULT_MINOR;
+                       pte_unmap(page_table);
+                       spin_unlock(&mm->page_table_lock);
+                       goto out;
+               }
+       } else {

---

 linux-2.6.9-rc4-taka/mm/memory.c   |    2 +-
 linux-2.6.9-rc4-taka/mm/mmigrate.c |   28 +++++++++++++++++++++-------
 linux-2.6.9-rc4-taka/mm/vmscan.c   |    4 ++++
 3 files changed, 26 insertions, 8 deletions
mm/mmigrate.c renamed from mm/mmigrate.c~marcelo-FIX1 +21 -7
diff -puN mm/mmigrate.c~marcelo-FIX1 mm/mmigrate.c
@@ -114,14 +114,14 @@ int migration_remove_entry(swp_entry_t e
 
 	lock_page(page);	
 
-	migration_remove_reference(page);
+	migration_remove_reference(page, 1);
 
 	unlock_page(page);
 
 	page_cache_release(page);
 }
 
-int migration_remove_reference(struct page *page)
+int migration_remove_reference(struct page *page, int dec)
 {
 	struct counter *c;
 	swp_entry_t entry;
@@ -134,10 +134,9 @@ int migration_remove_reference(struct pa
 
 	read_unlock_irq(&migration_space.tree_lock);
 
-	if (!c->i)
-		BUG();
+	BUG_ON(c->i < dec);
 
-	c->i--;
+	c->i -= dec;
 
 	if (!c->i) {
 		remove_from_migration_cache(page, page->private);
@@ -146,6 +145,15 @@ int migration_remove_reference(struct pa
 	}
 }
 
+int detach_from_migration_cache(struct page *page)
+{
+	lock_page(page);	
+	migration_remove_reference(page, 0);
+	unlock_page(page);
+
+	return 0;
+}
+
 int add_to_migration_cache(struct page *page, int gfp_mask) 
 {
 	int error, offset;
@@ -522,7 +530,9 @@ generic_migrate_page(struct page *page, 
 
 	/* map the newpage where the old page have been mapped. */
 	touch_unmapped_address(&vlist);
-	if (PageSwapCache(newpage)) {
+	if (PageMigration(newpage))
+		detach_from_migration_cache(newpage);
+	else if (PageSwapCache(newpage)) {
 		lock_page(newpage);
 		__remove_exclusive_swap_page(newpage, 1);
 		unlock_page(newpage);
@@ -538,7 +548,9 @@ out_busy:
 	/* Roll back all operations. */
 	unwind_page(page, newpage);
 	touch_unmapped_address(&vlist);
-	if (PageSwapCache(page)) {
+	if (PageMigration(page))
+		detach_from_migration_cache(page);
+	else if (PageSwapCache(page)) {
 		lock_page(page);
 		__remove_exclusive_swap_page(page, 1);
 		unlock_page(page);
@@ -550,6 +562,8 @@ out_removing:
 		BUG();
 	unlock_page(page);
 	unlock_page(newpage);
+	if (PageMigration(page))
+		detach_from_migration_cache(page);
 	return ret;
 }
 
mm/vmscan.c renamed from mm/vmscan.c~marcelo-FIX1 +4 -0
diff -puN mm/vmscan.c~marcelo-FIX1 mm/vmscan.c
@@ -459,6 +459,10 @@ int shrink_list(struct list_head *page_l
 			goto keep_locked;
 		}
 
+		if (PageMigration(page)) {
+			write_unlock_irq(&mapping->tree_lock);
+			goto keep_locked;
+		}
 #ifdef CONFIG_SWAP
 		if (PageSwapCache(page)) {
 			swp_entry_t swap = { .val = page->private };
mm/memory.c renamed from mm/memory.c~marcelo-FIX1 +1 -1
diff -puN mm/memory.c~marcelo-FIX1 mm/memory.c
@@ -1621,7 +1621,7 @@ again:
 		if (vm_swap_full())
 			remove_exclusive_swap_page(page);
 	} else {
-		migration_remove_reference(page);
+		migration_remove_reference(page, 1);
 	}
 
 	mm->rss++;

_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#26 lore raw
Quoted 6 lines · Hirokazu Takahashi

Hi Marcelo,

I've been testing the memory migration code with your patch. I found problems and I think the attached patch would fix some of them.

Quoted 7 lines

One of the problems is a race condition between add_to_migration_cache() and try_to_unmap(). Some pages in the migration cache cannot be removed with the current implementation. Please suppose
a process space might be removed between them. In this case no one can remove pages the process had from the migration cache, because they can be removed only when the pagetables pointed the pages.

I guess I dont fully understand you Hirokazu.

unmap_vmas function (called by exit_mmap) calls zap_pte_range, and that does:

                        if (pte_is_migration(pte)) {
                                migration_remove_entry(swp_entry);
                        } else
                                free_swap_and_cache(swp_entry);

migration_remove_entry should decrease the IDR counter, and remove the migration cache page on zero reference.

Am I missing something?

I assume you are seeing this problems in practice?

Sorry for the delay, been busy with other things.

Quoted 103 lines

Therefore, I made pages removed from the migration cache at the end of generic_migrate_page() if they remain in the cache.

The another is a fork() related problem. If fork() has occurred during page migration, the previous work may not go well. pages may not be removed from the migration cache.

So I made the swapcode ignore pages in the migration cache. However, as you know this is just a workaround and not a correct way to fix it.

Quoted 21 lines

Hi Hirokazu!

The problem is that another thread can fault in the pte (removing the radix tree entry) while the current thread dropped the page_table_lock - which explains the NULL lookup_migration_cache. The swap code handles this situation, but I've completly missed it.

Updated patch attached.

Extreme thanks for your testing, its being crucial!

We're getting there.

do_swap_page now does:

again:

(quoted diff) +4 -0
+       if (pte_is_migration(orig_pte)) {
+               page = lookup_migration_cache(entry.val);
+               if (!page) {
+                       spin_lock(&mm->page_table_lock);

Review

Quoted 10 lines
(quoted diff) +10 -0
+                       page_table = pte_offset_map(pmd, address);
+                       if (likely(pte_same(*page_table, orig_pte)))
+                               ret = VM_FAULT_OOM;
+                       else
+                               ret = VM_FAULT_MINOR;
+                       pte_unmap(page_table);
+                       spin_unlock(&mm->page_table_lock);
+                       goto out;
+               }
+       } else {

---

 linux-2.6.9-rc4-taka/mm/memory.c   |    2 +-
 linux-2.6.9-rc4-taka/mm/mmigrate.c |   28 +++++++++++++++++++++-------
 linux-2.6.9-rc4-taka/mm/vmscan.c   |    4 ++++
 3 files changed, 26 insertions, 8 deletions

diff -puN mm/mmigrate.c~marcelo-FIX1 mm/mmigrate.c

mm/mmigrate.c renamed from mm/mmigrate.c~marcelo-FIX1 +13 -5
@@ -114,14 +114,14 @@ int migration_remove_entry(swp_entry_t e
 
 	lock_page(page);	
 
-	migration_remove_reference(page);
+	migration_remove_reference(page, 1);
 
 	unlock_page(page);
 
 	page_cache_release(page);
 }
 
-int migration_remove_reference(struct page *page)
+int migration_remove_reference(struct page *page, int dec)
 {
 	struct counter *c;
 	swp_entry_t entry;
@@ -134,10 +134,9 @@ int migration_remove_reference(struct pa
 
 	read_unlock_irq(&migration_space.tree_lock);
 
-	if (!c->i)
-		BUG();
+	BUG_ON(c->i < dec);
 
-	c->i--;
+	c->i -= dec;
 
 	if (!c->i) {
 		remove_from_migration_cache(page, page->private);
@@ -146,6 +145,15 @@ int migration_remove_reference(struct pa
 	}
 }
 
+int detach_from_migration_cache(struct page *page)
+{
+	lock_page(page);	
+	migration_remove_reference(page, 0);
+	unlock_page(page);
+
+	return 0;
+}
+
 int add_to_migration_cache(struct page *page, int gfp_mask) 
 {
 	int error, offset;

mm/mmigrate.c generic_migrate_page(struct page *page, · line 538

Quoted 10 lines
mm/mmigrate.c renamed from mm/mmigrate.c~marcelo-FIX1 +3 -1
@@ -522,7 +530,9 @@ generic_migrate_page(struct page *page, 
 
 	/* map the newpage where the old page have been mapped. */
 	touch_unmapped_address(&vlist);
-	if (PageSwapCache(newpage)) {
+	if (PageMigration(newpage))
+		detach_from_migration_cache(newpage);
+	else if (PageSwapCache(newpage)) {
 		lock_page(newpage);
 		__remove_exclusive_swap_page(newpage, 1);
 		unlock_page(newpage);

I dont see this code on 2.6.9-rc2-mm4-mhp, I should upgrade.

Quoted 46 lines
mm/mmigrate.c renamed from mm/mmigrate.c~marcelo-FIX1 +5 -1
@@ -538,7 +548,9 @@ out_busy:
 	/* Roll back all operations. */
 	unwind_page(page, newpage);
 	touch_unmapped_address(&vlist);
-	if (PageSwapCache(page)) {
+	if (PageMigration(page))
+		detach_from_migration_cache(page);
+	else if (PageSwapCache(page)) {
 		lock_page(page);
 		__remove_exclusive_swap_page(page, 1);
 		unlock_page(page);
@@ -550,6 +562,8 @@ out_removing:
 		BUG();
 	unlock_page(page);
 	unlock_page(newpage);
+	if (PageMigration(page))
+		detach_from_migration_cache(page);
 	return ret;
 }
 

diff -puN mm/vmscan.c~marcelo-FIX1 mm/vmscan.c

mm/vmscan.c renamed from mm/vmscan.c~marcelo-FIX1 +4 -0
@@ -459,6 +459,10 @@ int shrink_list(struct list_head *page_l
 			goto keep_locked;
 		}
 
+		if (PageMigration(page)) {
+			write_unlock_irq(&mapping->tree_lock);
+			goto keep_locked;
+		}
 #ifdef CONFIG_SWAP
 		if (PageSwapCache(page)) {
 			swp_entry_t swap = { .val = page->private };

diff -puN mm/memory.c~marcelo-FIX1 mm/memory.c

mm/memory.c renamed from mm/memory.c~marcelo-FIX1 +1 -1
@@ -1621,7 +1621,7 @@ again:
 		if (vm_swap_full())
 			remove_exclusive_swap_page(page);
 	} else {
-		migration_remove_reference(page);
+		migration_remove_reference(page, 1);
 	}
 
 	mm->rss++;
Quoted 1 line

_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#27 lore raw

Hi Marcelo,

Quoted 24 lines
Quoted 6 lines

Hi Marcelo,

I've been testing the memory migration code with your patch. I found problems and I think the attached patch would fix some of them.

Quoted 7 lines

One of the problems is a race condition between add_to_migration_cache() and try_to_unmap(). Some pages in the migration cache cannot be removed with the current implementation. Please suppose
a process space might be removed between them. In this case no one can remove pages the process had from the migration cache, because they can be removed only when the pagetables pointed the pages.

I guess I dont fully understand you Hirokazu.

unmap_vmas function (called by exit_mmap) calls zap_pte_range, and that does:

                        if (pte_is_migration(pte)) {
                                migration_remove_entry(swp_entry);
                        } else
                                free_swap_and_cache(swp_entry);
Quoted 4 lines

migration_remove_entry should decrease the IDR counter, and remove the migration cache page on zero reference.

Am I missing something?

That's true only if the pte points a migration entry. However, the pte may not point it when zap_pte_range() is called in some case.

Please suppose the following flow.
Any process may exit or munmap during memory migration before calling set_pte(migration entry). This will keep some unreferenced pages in the migration cache. No one can remove these pages.

<start page migration> <Process A>

        |                                      |
        |                                      |
        |                                      |
 add_to_migration_cache()                      |
    insert a page of Process A  ----------->   |
    in the migration cache.                    |
        |                                      |
        |                               zap_pte_range()
        |                   X <------------ migration_remove_entry()
        |                      the pte associated with the page doesn't
        |                      point any migration entries.
        |
        |
 try_to_unmap() -----------------------> X
     migration_duplicate()       no pte mapping the page can be found.
     set_pte(migration entry)
        |
        |

migrate_fn()
|
|
<finish>
the page still remains in the migration cache. the page may be referred by no process.

Quoted 1 line

I assume you are seeing this problems in practice?

Yes, it often happens without the patch.

Quoted 1 line

Sorry for the delay, been busy with other things.

No problem. Everyone knows you're doing hard work!

Quoted 161 lines
Quoted 103 lines

Therefore, I made pages removed from the migration cache at the end of generic_migrate_page() if they remain in the cache.

The another is a fork() related problem. If fork() has occurred during page migration, the previous work may not go well. pages may not be removed from the migration cache.

So I made the swapcode ignore pages in the migration cache. However, as you know this is just a workaround and not a correct way to fix it.

Quoted 21 lines

Hi Hirokazu!

The problem is that another thread can fault in the pte (removing the radix tree entry) while the current thread dropped the page_table_lock - which explains the NULL lookup_migration_cache. The swap code handles this situation, but I've completly missed it.

Updated patch attached.

Extreme thanks for your testing, its being crucial!

We're getting there.

do_swap_page now does:

again:

(quoted diff) +4 -0
+       if (pte_is_migration(orig_pte)) {
+               page = lookup_migration_cache(entry.val);
+               if (!page) {
+                       spin_lock(&mm->page_table_lock);

Review

Quoted 10 lines
(quoted diff) +10 -0
+                       page_table = pte_offset_map(pmd, address);
+                       if (likely(pte_same(*page_table, orig_pte)))
+                               ret = VM_FAULT_OOM;
+                       else
+                               ret = VM_FAULT_MINOR;
+                       pte_unmap(page_table);
+                       spin_unlock(&mm->page_table_lock);
+                       goto out;
+               }
+       } else {

---

 linux-2.6.9-rc4-taka/mm/memory.c   |    2 +-
 linux-2.6.9-rc4-taka/mm/mmigrate.c |   28 +++++++++++++++++++++-------
 linux-2.6.9-rc4-taka/mm/vmscan.c   |    4 ++++
 3 files changed, 26 insertions, 8 deletions

diff -puN mm/mmigrate.c~marcelo-FIX1 mm/mmigrate.c

mm/mmigrate.c renamed from mm/mmigrate.c~marcelo-FIX1 +13 -5
@@ -114,14 +114,14 @@ int migration_remove_entry(swp_entry_t e
 
 	lock_page(page);	
 
-	migration_remove_reference(page);
+	migration_remove_reference(page, 1);
 
 	unlock_page(page);
 
 	page_cache_release(page);
 }
 
-int migration_remove_reference(struct page *page)
+int migration_remove_reference(struct page *page, int dec)
 {
 	struct counter *c;
 	swp_entry_t entry;
@@ -134,10 +134,9 @@ int migration_remove_reference(struct pa
 
 	read_unlock_irq(&migration_space.tree_lock);
 
-	if (!c->i)
-		BUG();
+	BUG_ON(c->i < dec);
 
-	c->i--;
+	c->i -= dec;
 
 	if (!c->i) {
 		remove_from_migration_cache(page, page->private);
@@ -146,6 +145,15 @@ int migration_remove_reference(struct pa
 	}
 }
 
+int detach_from_migration_cache(struct page *page)
+{
+	lock_page(page);	
+	migration_remove_reference(page, 0);
+	unlock_page(page);
+
+	return 0;
+}
+
 int add_to_migration_cache(struct page *page, int gfp_mask) 
 {
 	int error, offset;

mm/mmigrate.c generic_migrate_page(struct page *page, · line 538

Quoted 10 lines
mm/mmigrate.c renamed from mm/mmigrate.c~marcelo-FIX1 +3 -1
@@ -522,7 +530,9 @@ generic_migrate_page(struct page *page, 
 
 	/* map the newpage where the old page have been mapped. */
 	touch_unmapped_address(&vlist);
-	if (PageSwapCache(newpage)) {
+	if (PageMigration(newpage))
+		detach_from_migration_cache(newpage);
+	else if (PageSwapCache(newpage)) {
 		lock_page(newpage);
 		__remove_exclusive_swap_page(newpage, 1);
 		unlock_page(newpage);

I dont see this code on 2.6.9-rc2-mm4-mhp, I should upgrade.

Quoted 45 lines
mm/mmigrate.c renamed from mm/mmigrate.c~marcelo-FIX1 +5 -1
@@ -538,7 +548,9 @@ out_busy:
 	/* Roll back all operations. */
 	unwind_page(page, newpage);
 	touch_unmapped_address(&vlist);
-	if (PageSwapCache(page)) {
+	if (PageMigration(page))
+		detach_from_migration_cache(page);
+	else if (PageSwapCache(page)) {
 		lock_page(page);
 		__remove_exclusive_swap_page(page, 1);
 		unlock_page(page);
@@ -550,6 +562,8 @@ out_removing:
 		BUG();
 	unlock_page(page);
 	unlock_page(newpage);
+	if (PageMigration(page))
+		detach_from_migration_cache(page);
 	return ret;
 }
 

diff -puN mm/vmscan.c~marcelo-FIX1 mm/vmscan.c

mm/vmscan.c renamed from mm/vmscan.c~marcelo-FIX1 +4 -0
@@ -459,6 +459,10 @@ int shrink_list(struct list_head *page_l
 			goto keep_locked;
 		}
 
+		if (PageMigration(page)) {
+			write_unlock_irq(&mapping->tree_lock);
+			goto keep_locked;
+		}
 #ifdef CONFIG_SWAP
 		if (PageSwapCache(page)) {
 			swp_entry_t swap = { .val = page->private };

diff -puN mm/memory.c~marcelo-FIX1 mm/memory.c

mm/memory.c renamed from mm/memory.c~marcelo-FIX1 +1 -1
@@ -1621,7 +1621,7 @@ again:
 		if (vm_swap_full())
 			remove_exclusive_swap_page(page);
 	} else {
-		migration_remove_reference(page);
+		migration_remove_reference(page, 1);
 	}
 
Quoted 3 lines
Quoted 2 lines
mm/memory.c renamed from mm/memory.c~marcelo-FIX1 +0 -0
 	mm->rss++;

_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#28 lore raw
Quoted 1 line · Hirokazu Takahashi

Hi Marcelo,

Hi again Hirokazu, finally found sometime to think about this.

Quoted 40 lines
Quoted 24 lines
Quoted 6 lines

Hi Marcelo,

I've been testing the memory migration code with your patch. I found problems and I think the attached patch would fix some of them.

Quoted 7 lines

One of the problems is a race condition between add_to_migration_cache() and try_to_unmap(). Some pages in the migration cache cannot be removed with the current implementation. Please suppose
a process space might be removed between them. In this case no one can remove pages the process had from the migration cache, because they can be removed only when the pagetables pointed the pages.

I guess I dont fully understand you Hirokazu.

unmap_vmas function (called by exit_mmap) calls zap_pte_range, and that does:

                        if (pte_is_migration(pte)) {
                                migration_remove_entry(swp_entry);
                        } else
                                free_swap_and_cache(swp_entry);
Quoted 4 lines

migration_remove_entry should decrease the IDR counter, and remove the migration cache page on zero reference.

Am I missing something?

That's true only if the pte points a migration entry. However, the pte may not point it when zap_pte_range() is called in some case.

Please suppose the following flow.
Any process may exit or munmap during memory migration before calling set_pte(migration entry). This will keep some unreferenced pages in the migration cache. No one can remove these pages.

<start page migration> <Process A>

Quoted 11 lines
        |                                      |
        |                                      |
        |                                      |
 add_to_migration_cache()                      |
    insert a page of Process A  ----------->   |
    in the migration cache.                    |
        |                                      |
        |                               zap_pte_range()
        |                   X <------------ migration_remove_entry()
        |                      the pte associated with the page doesn't
        |                      point any migration entries.

OK, I see it, its the "normal" anonymous pte which will be removed at this point.

Quoted 15 lines
        |
        |
 try_to_unmap() -----------------------> X
     migration_duplicate()       no pte mapping the page can be found.
     set_pte(migration entry)
        |
        |

migrate_fn()
|
|
<finish>
the page still remains in the migration cache. the page may be referred by no process.

Quoted 10 lines
Quoted 1 line

I assume you are seeing this problems in practice?

Yes, it often happens without the patch.

Quoted 1 line

Sorry for the delay, been busy with other things.

No problem. Everyone knows you're doing hard work!

Quoted 2 lines
Quoted 2 lines

Therefore, I made pages removed from the migration cache at the end of generic_migrate_page() if they remain in the cache.

OK, removing migration pages at end of generic_migrate_page() should avoid the leak - that part of your patch is fine to me!

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

The another is a fork() related problem. If fork() has occurred during page migration, the previous work may not go well. pages may not be removed from the migration cache.

Can you please expand on that one? I assume it works fine because copy_page_range() duplicates the migration page reference (and the migration pte), meaning that on exit (zap_pte_range) the migration pages should be removed through migration_remove_entry().

I dont see the problem - please correct me.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

So I made the swapcode ignore pages in the migration cache. However, as you know this is just a workaround and not a correct way to fix it.

What this has to do with fork()? I can't understand.

Your patch is correct here also - we can't reclaim migration cache pages.

+ if (PageMigration(page)) {
+ write_unlock_irq(&mapping->tree_lock);
+ goto keep_locked;
+ }

An enhancement would be to force pagefault of all pte's mapping to a migration cache page on shrink_list.

similar to rmap.c's try_to_unmap_anon() but intented to create the pte instead of unmapping it

        anon_vma = page_lock_anon_vma(page);

        list_for_each_entry(vma, &anon_vma->head, anon_vma_node)
		ret = try_to_faultin(page, vma);

And try_to_faultin() calling handle_mm_fault()...

Is that what you mean?

Anyways, does the migration cache survive your stress testing now with these changes ?

I've coded the beginning of skeleton for the nonblocking version of migrate_onepage().

Can you generate a new migration cache patch on top of linux-2.6.10-rc1-mm2-mhp2 with your fixes ?

Thanks!
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#29 lore raw

Hi Marcelo,

Sorry for the delayed reply.

Quoted 82 lines
Quoted 38 lines
Quoted 22 lines
Quoted 11 lines

I've been testing the memory migration code with your patch. I found problems and I think the attached patch would fix some of them.

One of the problems is a race condition between add_to_migration_cache() and try_to_unmap(). Some pages in the migration cache cannot be removed with the current implementation. Please suppose
a process space might be removed between them. In this case no one can remove pages the process had from the migration cache, because they can be removed only when the pagetables pointed the pages.

I guess I dont fully understand you Hirokazu.

unmap_vmas function (called by exit_mmap) calls zap_pte_range, and that does:

                        if (pte_is_migration(pte)) {
                                migration_remove_entry(swp_entry);
                        } else
                                free_swap_and_cache(swp_entry);
Quoted 4 lines

migration_remove_entry should decrease the IDR counter, and remove the migration cache page on zero reference.

Am I missing something?

That's true only if the pte points a migration entry. However, the pte may not point it when zap_pte_range() is called in some case.

Please suppose the following flow.
Any process may exit or munmap during memory migration before calling set_pte(migration entry). This will keep some unreferenced pages in the migration cache. No one can remove these pages.

<start page migration> <Process A>

Quoted 11 lines
        |                                      |
        |                                      |
        |                                      |
 add_to_migration_cache()                      |
    insert a page of Process A  ----------->   |
    in the migration cache.                    |
        |                                      |
        |                               zap_pte_range()
        |                   X <------------ migration_remove_entry()
        |                      the pte associated with the page doesn't
        |                      point any migration entries.

OK, I see it, its the "normal" anonymous pte which will be removed at this point.

Quoted 15 lines
        |
        |
 try_to_unmap() -----------------------> X
     migration_duplicate()       no pte mapping the page can be found.
     set_pte(migration entry)
        |
        |

migrate_fn()
|
|
<finish>
the page still remains in the migration cache. the page may be referred by no process.

Quoted 10 lines
Quoted 1 line

I assume you are seeing this problems in practice?

Yes, it often happens without the patch.

Quoted 1 line

Sorry for the delay, been busy with other things.

No problem. Everyone knows you're doing hard work!

Quoted 2 lines
Quoted 2 lines

Therefore, I made pages removed from the migration cache at the end of generic_migrate_page() if they remain in the cache.

OK, removing migration pages at end of generic_migrate_page() should avoid the leak - that part of your patch is fine to me!

Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

The another is a fork() related problem. If fork() has occurred during page migrationa, the previous work may not go well. pages may not be removed from the migration cache.

Can you please expand on that one? I assume it works fine because copy_page_range() duplicates the migration page reference (and the migration pte), meaning that on exit (zap_pte_range) the migration pages should be removed through migration_remove_entry().

Yes, that's true.

Quoted 1 line

I dont see the problem - please correct me.

However, once the page is moved into the migration cache, no one can make it swapped out. This problem may be solved by your approach described below.

Quoted 5 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

So I made the swapcode ignore pages in the migration cache. However, as you know this is just a workaround and not a correct way to fix it.

What this has to do with fork()? I can't understand.

fork() may leave some pages in the migration cache with my latest implementation, though the memory migration code tries to remove them from the migration cache by forcible pagefault in touch_unmapped_address().

However, touch_unmapped_address() doesn't know that the migration page has been duplicated.

Quoted 8 lines

Your patch is correct here also - we can't reclaim migration cache pages.

(quoted diff) +4 -0
+	if (PageMigration(page)) {
+		write_unlock_irq(&mapping->tree_lock);
+		goto keep_locked;
+	}
Quoted 5 lines

An enhancement would be to force pagefault of all pte's mapping to a migration cache page on shrink_list.

similar to rmap.c's try_to_unmap_anon() but intented to create the pte instead of unmapping it

If it works as we expect, this code can be called at the end of generic_migrate_page() I guess.

Quoted 11 lines
        anon_vma = page_lock_anon_vma(page);

        list_for_each_entry(vma, &anon_vma->head, anon_vma_node)
		ret = try_to_faultin(page, vma);

And try_to_faultin() calling handle_mm_fault()...

Is that what you mean?

Anyways, does the migration cache survive your stress testing now with these changes ?

Sure.

Quoted 4 lines

I've coded the beginning of skeleton for the nonblocking version of migrate_onepage().

Can you generate a new migration cache patch on top of linux-2.6.10-rc1-mm2-mhp2 with your fixes ?

I ported your patch and my fixes on the top of linux-2.6.10-rc1-mm5-mhp1.

Thanks,
Hirokazu Takahashi.

---

---

 linux-2.6.10-rc1-mm5-taka/include/linux/mm.h      |   23 ++
 linux-2.6.10-rc1-mm5-taka/include/linux/swap.h    |   16 +
 linux-2.6.10-rc1-mm5-taka/include/linux/swapops.h |   25 ++
 linux-2.6.10-rc1-mm5-taka/mm/fremap.c             |   11 -
 linux-2.6.10-rc1-mm5-taka/mm/memory.c             |   50 ++++-
 linux-2.6.10-rc1-mm5-taka/mm/mmigrate.c           |  192 +++++++++++++++++++++-
 linux-2.6.10-rc1-mm5-taka/mm/page_io.c            |    1 
 linux-2.6.10-rc1-mm5-taka/mm/rmap.c               |   32 ++-
 linux-2.6.10-rc1-mm5-taka/mm/shmem.c              |    1 
 linux-2.6.10-rc1-mm5-taka/mm/swapfile.c           |    9 -
 linux-2.6.10-rc1-mm5-taka/mm/vmscan.c             |    6 
 11 files changed, 331 insertions, 35 deletions
include/linux/mm.h renamed from include/linux/mm.h~migration_cache_marcelo5 +22 -1
diff -puN include/linux/mm.h~migration_cache_marcelo5 include/linux/mm.h
@@ -286,6 +286,24 @@ extern int capture_page_range(unsigned l
  * files which need it (119 of them)
  */
 #include <linux/page-flags.h>
+#include <linux/swap.h>
+#include <linux/swapops.h> 
+
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
+
 
 /*
  * Methods to modify the page usage count.
@@ -493,11 +511,14 @@ void page_address_init(void);
 #define PAGE_MAPPING_ANON	1
 
 extern struct address_space swapper_space;
+extern struct address_space migration_space;
 static inline struct address_space *page_mapping(struct page *page)
 {
 	struct address_space *mapping = page->mapping;
 
-	if (unlikely(PageSwapCache(page)))
+	if (unlikely(PageMigration(page)))
+		mapping = &migration_space;
+	else if (unlikely(PageSwapCache(page)))
 		mapping = &swapper_space;
 	else if (unlikely((unsigned long)mapping & PAGE_MAPPING_ANON))
 		mapping = NULL;
include/linux/swap.h renamed from include/linux/swap.h~migration_cache_marcelo5 +16 -0
diff -puN include/linux/swap.h~migration_cache_marcelo5 include/linux/swap.h
@@ -258,6 +258,7 @@ static inline int remove_exclusive_swap_
 {
 	return __remove_exclusive_swap_page(p, 0);
 }
+extern int migration_remove_entry(swp_entry_t);
 struct backing_dev_info;
 
 extern struct swap_list_t swap_list;
@@ -331,6 +332,21 @@ static inline swp_entry_t get_swap_page(
 #define put_swap_token(x) do { } while(0)
 #define grab_swap_token()  do { } while(0)
 #define has_swap_token(x) 0
+
+static inline int PageMigration(struct page *page)
+{
+        swp_entry_t entry;
+
+        if (!PageSwapCache(page))
+                return 0;
+
+        entry.val = page->private;
+
+        if (swp_type(entry) != MIGRATION_TYPE)
+                return 0;
+
+        return 1;
+}
 
 #endif /* CONFIG_SWAP */
 #endif /* __KERNEL__*/
include/linux/swapops.h renamed from include/linux/swapops.h~migration_cache_marcelo5 +24 -1
diff -puN include/linux/swapops.h~migration_cache_marcelo5 include/linux/swapops.h
@@ -10,7 +10,9 @@
  * swp_entry_t's are *never* stored anywhere in their arch-dependent format.
  */
 #define SWP_TYPE_SHIFT(e)	(sizeof(e.val) * 8 - MAX_SWAPFILES_SHIFT)
-#define SWP_OFFSET_MASK(e)	((1UL << SWP_TYPE_SHIFT(e)) - 1)
+#define SWP_OFFSET_MASK(e)	((1UL << (SWP_TYPE_SHIFT(e))) - 1)
+
+#define MIGRATION_TYPE  (MAX_SWAPFILES - 1)
 
 /*
  * Store a type+offset into a swp_entry_t in an arch-independent format
@@ -67,3 +69,24 @@ static inline pte_t swp_entry_to_pte(swp
 	BUG_ON(pte_file(__swp_entry_to_pte(arch_entry)));
 	return __swp_entry_to_pte(arch_entry);
 }
+
+static inline int pte_is_migration(pte_t pte)
+{
+	unsigned long swp_type;
+	swp_entry_t arch_entry;
+
+	arch_entry = __pte_to_swp_entry(pte);
+
+	swp_type = __swp_type(arch_entry);
+
+	return swp_type == MIGRATION_TYPE;
+}
+
+static inline pte_t migration_entry_to_pte(swp_entry_t entry)
+{
+	swp_entry_t arch_entry;
+	
+	arch_entry = __swp_entry(MIGRATION_TYPE, swp_offset(entry));
+	return __swp_entry_to_pte(arch_entry);
+}
+
mm/fremap.c renamed from mm/fremap.c~migration_cache_marcelo5 +8 -3
diff -puN mm/fremap.c~migration_cache_marcelo5 mm/fremap.c
@@ -11,7 +11,6 @@
 #include <linux/file.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
-#include <linux/swapops.h>
 #include <linux/rmap.h>
 #include <linux/module.h>
 #include <linux/syscalls.h>
@@ -43,8 +42,14 @@ static inline void zap_pte(struct mm_str
 			}
 		}
 	} else {
-		if (!pte_file(pte))
-			free_swap_and_cache(pte_to_swp_entry(pte));
+		if (!pte_file(pte)) {
+			swp_entry_t swp_entry = pte_to_swp_entry(pte);
+			if (pte_is_migration(pte)) { 
+				migration_remove_entry(swp_entry);
+			} else {
+				free_swap_and_cache(swp_entry);
+			}
+		}
 		pte_clear(ptep);
 	}
 }
mm/memory.c renamed from mm/memory.c~migration_cache_marcelo5 +41 -9

94 lines not on this page · raw message

mm/mmigrate.c renamed from mm/mmigrate.c~migration_cache_marcelo5 +185 -7

248 lines not on this page · raw message

mm/page_io.c renamed from mm/page_io.c~migration_cache_marcelo5 +0 -1
diff -puN mm/page_io.c~migration_cache_marcelo5 mm/page_io.c
@@ -15,7 +15,6 @@
 #include <linux/pagemap.h>
 #include <linux/swap.h>
 #include <linux/bio.h>
-#include <linux/swapops.h>
 #include <linux/writeback.h>
 #include <asm/pgtable.h>
 
mm/rmap.c renamed from mm/rmap.c~migration_cache_marcelo5 +23 -9
diff -puN mm/rmap.c~migration_cache_marcelo5 mm/rmap.c
@@ -49,7 +49,7 @@
 #include <linux/sched.h>
 #include <linux/pagemap.h>
 #include <linux/swap.h>
-#include <linux/swapops.h>
+//#include <linux/swapops.h>
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/acct.h>
@@ -655,15 +655,29 @@ static int try_to_unmap_one(struct page 
 		 * Store the swap location in the pte.
 		 * See handle_pte_fault() ...
 		 */
-		BUG_ON(!PageSwapCache(page));
-		swap_duplicate(entry);
-		if (list_empty(&mm->mmlist)) {
-			spin_lock(&mmlist_lock);
-			list_add(&mm->mmlist, &init_mm.mmlist);
-			spin_unlock(&mmlist_lock);
+		//BUG_ON(!PageSwapCache(page));
+		if (PageSwapCache(page) && !PageMigration(page)) {
+			swap_duplicate(entry);
+			if (list_empty(&mm->mmlist)) {
+				spin_lock(&mmlist_lock);
+				list_add(&mm->mmlist, &init_mm.mmlist);
+				spin_unlock(&mmlist_lock);
+			}
+			set_pte(pte, swp_entry_to_pte(entry));
+			BUG_ON(pte_file(*pte));
+		} else if (PageMigration(page)) {
+			// page cache get to reference pte,
+			// remove from migration cache
+			// on zero-users at fault path
+			migration_duplicate(entry);
+			if (list_empty(&mm->mmlist)) {
+				spin_lock(&mmlist_lock);
+				list_add(&mm->mmlist, &init_mm.mmlist);
+				spin_unlock(&mmlist_lock);
+			}
+			set_pte(pte, migration_entry_to_pte(entry));
+			BUG_ON(pte_file(*pte));
 		}
-		set_pte(pte, swp_entry_to_pte(entry));
-		BUG_ON(pte_file(*pte));
 		mm->anon_rss--;
 	}
 
mm/shmem.c renamed from mm/shmem.c~migration_cache_marcelo5 +0 -1
diff -puN mm/shmem.c~migration_cache_marcelo5 mm/shmem.c
@@ -42,7 +42,6 @@
 #include <linux/vfs.h>
 #include <linux/blkdev.h>
 #include <linux/security.h>
-#include <linux/swapops.h>
 #include <linux/mempolicy.h>
 #include <linux/namei.h>
 #include <linux/xattr.h>
mm/swapfile.c renamed from mm/swapfile.c~migration_cache_marcelo5 +8 -1
diff -puN mm/swapfile.c~migration_cache_marcelo5 mm/swapfile.c
@@ -34,7 +34,6 @@
 
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
-#include <linux/swapops.h>
 
 spinlock_t swaplock = SPIN_LOCK_UNLOCKED;
 unsigned int nr_swapfiles;
@@ -235,6 +234,7 @@ bad_device:
 	printk(KERN_ERR "swap_free: %s%08lx\n", Unused_file, entry.val);
 	goto out;
 bad_nofile:
+	BUG();
 	printk(KERN_ERR "swap_free: %s%08lx\n", Bad_file, entry.val);
 out:
 	return NULL;
@@ -1409,6 +1409,13 @@ asmlinkage long sys_swapon(const char __
 		swap_list_unlock();
 		goto out;
 	}
+
+	/* MIGRATION_TYPE is reserved for migration pages */
+	if (type >= MIGRATION_TYPE) {
+		swap_list_unlock();
+		goto out;
+	}
+
 	if (type >= nr_swapfiles)
 		nr_swapfiles = type+1;
 	INIT_LIST_HEAD(&p->extent_list);
mm/vmscan.c renamed from mm/vmscan.c~migration_cache_marcelo5 +4 -2
diff -puN mm/vmscan.c~migration_cache_marcelo5 mm/vmscan.c
@@ -38,8 +38,6 @@
 #include <asm/tlbflush.h>
 #include <asm/div64.h>
 
-#include <linux/swapops.h>
-
 /*
  * The list of shrinker callbacks used by to apply pressure to
  * ageable caches.
@@ -458,6 +456,10 @@ int shrink_list(struct list_head *page_l
 			goto keep_locked;
 		}
 
+		if (PageMigration(page)) {
+			write_unlock_irq(&mapping->tree_lock);
+			goto keep_locked;
+		}
 #ifdef CONFIG_SWAP
 		if (PageSwapCache(page)) {
 			swp_entry_t swap = { .val = page->private };

_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#30 lore raw

Hi Hirokazu,

Quoted 32 lines · Hirokazu Takahashi

Hi Marcelo,

Quoted 10 lines
Quoted 6 lines
Quoted 1 line

Sorry for the delay, been busy with other things.

No problem. Everyone knows you're doing hard work!

Quoted 2 lines
Quoted 2 lines

Therefore, I made pages removed from the migration cache at the end of generic_migrate_page() if they remain in the cache.

OK, removing migration pages at end of generic_migrate_page() should avoid the leak - that part of your patch is fine to me!

Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

The another is a fork() related problem. If fork() has occurred during page migrationa, the previous work may not go well. pages may not be removed from the migration cache.

Can you please expand on that one? I assume it works fine because copy_page_range() duplicates the migration page reference (and the migration pte), meaning that on exit (zap_pte_range) the migration pages should be removed through migration_remove_entry().

Yes, that's true.

Quoted 1 line

I dont see the problem - please correct me.

However, once the page is moved into the migration cache, no one can make it swapped out. This problem may be solved by your approach described below.

Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

So I made the swapcode ignore pages in the migration cache. However, as you know this is just a workaround and not a correct way to fix it.

Quoted 6 lines
Quoted 1 line

What this has to do with fork()? I can't understand.

fork() may leave some pages in the migration cache with my latest implementation, though the memory migration code tries to remove them from the migration cache by forcible pagefault in touch_unmapped_address().

Why are record_unmapped_address/touch_unmapped_address needed ?

I started investigating the issue which migration pages couldnt be swapped out, but found out that migration pages are never left in the cache because touch_unmapped_address recreates the ptes removing the pages from the migration cache.

That means we have no problem with migration cache pages left pinned (unswappable) in memory, which means it is fully functional AFAICT.

However, I thought the intent was to fault the pages on demand?

I even wrote this to be called at vmscan() time but touch_unmapped_address already has similar functionality at migration time.

int try_to_faultin(struct page *page)

{
        struct anon_vma *anon_vma;
        struct vm_area_struct *vma;
        unsigned long address;
        int ret = 0;

restart:

        anon_vma = page_lock_anon_vma(page);
        if (!anon_vma)
                return ret;

        list_for_each_entry(vma, &anon_vma->head, anon_vma_node) {
                address = vma_address(page, vma);
                // handle if (address = -EFAULT) ? 
                if (!follow_and_check_present(vma->vm_mm, address))
                        continue;

                spin_unlock(&anon_vma->lock);
                switch (handle_mm_fault(vma->vm_mm, vma, address, 0)) {
                case VM_FAULT_MINOR:
                        goto restart;
                case VM_FAULT_MAJOR:
                        BUG();
                case VM_FAULT_SIGBUS:
                case VM_FAULT_OOM:
                        goto out_unlock;
                }
        }
        ret = 1;
        printk(KERN_ERR "faulted migration page in!\n");

out_unlock:

        spin_unlock(&anon_vma->lock);
        return ret;

}

Quoted 12 lines

However, touch_unmapped_address() doesn't know that the migration page has been duplicated.

Quoted 8 lines

Your patch is correct here also - we can't reclaim migration cache pages.

mm/vmscan.c renamed from mm/vmscan.c~migration_cache_marcelo5 +4 -0
+	if (PageMigration(page)) {
+		write_unlock_irq(&mapping->tree_lock);
+		goto keep_locked;
+	}
Quoted 8 lines
Quoted 5 lines

An enhancement would be to force pagefault of all pte's mapping to a migration cache page on shrink_list.

similar to rmap.c's try_to_unmap_anon() but intented to create the pte instead of unmapping it

If it works as we expect, this code can be called at the end of generic_migrate_page() I guess.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#31 lore raw

Marcello and Hirokazu,

I've finally gotten around to working on my user controlled page migration project. (What I'm trying to implement is a way for a suitably authorized user program to request that some or all of the pages or a particular address
space be migrated from one NUMA node to another.)

The first such thing to try to migrate is anonymous, private pages so that is what I am working on. To keep things simple, the user program is trying to migrate part of its own address space.

What I have found out is that this works correctly with the page-migration patch (extracted from the memory hotplug patch, e. g.:

http://sr71.net/patches/2.6.10/2.6.10-mm1-mhp-test7/page_migration/

) but when I apply the migration cache patch on top what happens is that the migration appears to occur, but then as soon as I return from the system call, I get:

VM: killing process test
Killed

(This message comes from ia64_do_page_fault() and appears to because handle_mm_fault() returned FAULT_OOM....)

I haven't looked into this further, but was wondering if perhaps one of you would understand why the migrate cache patch would fail in this way?

Thanks,

Signature · 14 lines
Best Regards,
Ray
-----------------------------------------------
                  Ray Bryant
512-453-9679 (work)         512-507-7807 (cell)
[email protected]             [email protected]
The box said: "Requires Windows 98 or better",
           so I installed Linux.
-----------------------------------------------
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#32 lore raw
Quoted 21 lines · Ray Bryant

Marcello and Hirokazu,

I've finally gotten around to working on my user controlled page migration project. (What I'm trying to implement is a way for a suitably authorized user program to request that some or all of the pages or a particular address
space be migrated from one NUMA node to another.)

The first such thing to try to migrate is anonymous, private pages so that is what I am working on. To keep things simple, the user program is trying to migrate part of its own address space.

What I have found out is that this works correctly with the page-migration patch (extracted from the memory hotplug patch, e. g.:

http://sr71.net/patches/2.6.10/2.6.10-mm1-mhp-test7/page_migration/

) but when I apply the migration cache patch on top what happens is that the migration appears to occur, but then as soon as I return from the system call, I get:

Quoted 8 lines

VM: killing process test
Killed

(This message comes from ia64_do_page_fault() and appears to because handle_mm_fault() returned FAULT_OOM....)

I haven't looked into this further, but was wondering if perhaps one of you would understand why the migrate cache patch would fail in this way?

I can't think of anything right now - probably do_wp_page() is returning FAULT_OOM, can you confirm that?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#33 lore raw
Quoted 5 lines · Marcelo Tosatti
Quoted 5 lines

(This message comes from ia64_do_page_fault() and appears to because handle_mm_fault() returned FAULT_OOM....)

Quoted 7 lines
Quoted 2 lines

I haven't looked into this further, but was wondering if perhaps one of you would understand why the migrate cache patch would fail in this way?

I can't think of anything right now - probably do_wp_page() is returning FAULT_OOM, can you confirm that?

No, it doesn't appear to be do_wp_page(). It looks like get_swap_page() returns FAULT_OOM followed by get_user_pages() returning FAULT_OOM. For the page that causes the VM to kill the process, there is no return
from get_user_pages() that returns FAULT_OOM. Not sure yet what is going
on here.

Signature · 12 lines
-----------------------------------------------
Ray Bryant
512-453-9679 (work)         512-507-7807 (cell)
[email protected]             [email protected]
The box said: "Requires Windows 98 or better",
	 so I installed Linux.
-----------------------------------------------
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#34 lore raw

Hi Ray,

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines

(This message comes from ia64_do_page_fault() and appears to because handle_mm_fault() returned FAULT_OOM....)

I haven't looked into this further, but was wondering if perhaps one of you would understand why the migrate cache patch would fail in this way?

Quoted 8 lines
Quoted 3 lines

I can't think of anything right now - probably do_wp_page() is returning FAULT_OOM, can you confirm that?

No, it doesn't appear to be do_wp_page(). It looks like get_swap_page() returns FAULT_OOM followed by get_user_pages() returning FAULT_OOM. For the page that causes the VM to kill the process, there is no return from get_user_pages() that returns FAULT_OOM. Not sure yet what is going on here.

The current implementation requires swap devices to migrate pages. Have you added any swap devices?

This restriction will be solved with the migration cache Marcelo is working on.

Thanks,
Hirokazu Takahashi.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#35 lore raw
Quoted 20 lines · Hirokazu Takahashi

Hi Ray,

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines

(This message comes from ia64_do_page_fault() and appears to because handle_mm_fault() returned FAULT_OOM....)

I haven't looked into this further, but was wondering if perhaps one of you would understand why the migrate cache patch would fail in this way?

Quoted 8 lines
Quoted 3 lines

I can't think of anything right now - probably do_wp_page() is returning FAULT_OOM, can you confirm that?

No, it doesn't appear to be do_wp_page(). It looks like get_swap_page() returns FAULT_OOM followed by get_user_pages() returning FAULT_OOM. For the page that causes the VM to kill the process, there is no return
from get_user_pages() that returns FAULT_OOM. Not sure yet what is going
on here.

Quoted 10 lines

The current implementation requires swap devices to migrate pages. Have you added any swap devices?

This restriction will be solved with the migration cache Marcelo is working on.

Thanks,
Hirokazu Takahashi.

I'm running with the migration cache patch applied as well. This is a requirement for the project I am working on as the customer doesn't want to swap out pages just to migrated them.

If I take out the migration cache patch, this "VM: killing ..." problem
goes away. So it has something to do specifically with the migration
cache code.

Signature · 12 lines
-----------------------------------------------
Ray Bryant
512-453-9679 (work)         512-507-7807 (cell)
[email protected]             [email protected]
The box said: "Requires Windows 98 or better",
	 so I installed Linux.
-----------------------------------------------
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#36 lore raw

Hi Ray,

I realized the situation.

Quoted 24 lines
Quoted 24 lines
Quoted 11 lines
Quoted 10 lines
Quoted 5 lines

(This message comes from ia64_do_page_fault() and appears to because handle_mm_fault() returned FAULT_OOM....)

I haven't looked into this further, but was wondering if perhaps one of you would understand why the migrate cache patch would fail in this way?

I can't think of anything right now - probably do_wp_page() is returning FAULT_OOM, can you confirm that?

Quoted 5 lines

No, it doesn't appear to be do_wp_page(). It looks like get_swap_page() returns FAULT_OOM followed by get_user_pages() returning FAULT_OOM. For the page that causes the VM to kill the process, there is no return from get_user_pages() that returns FAULT_OOM. Not sure yet what is going on here.

The current implementation requires swap devices to migrate pages. Have you added any swap devices?

This restriction will be solved with the migration cache Marcelo is working on.

Quoted 7 lines
Quoted 4 lines

Thanks,
Hirokazu Takahashi.

I'm running with the migration cache patch applied as well. This is a requirement for the project I am working on as the customer doesn't want to swap out pages just to migrated them.

I see.

Quoted 3 lines

If I take out the migration cache patch, this "VM: killing ..." problem
goes away. So it has something to do specifically with the migration
cache code.

I've never seen the message though the migration cache code may have some bugs. May I ask you some questions about it?

- Which version of kernel did you use for it?
- Which migration cache code did you choose?
- How many nodes, CPUs and memory does your box have?
- What kind of applications were running on your box?
- How often did this happened?
- Did this message appear right after starting the migration? Or it appeared some short while later?
- How the target pages to be migrated were selected?
- How did you kick memory migration started?
- Please show me /proc/meminfo when the problem happened.
- Is it possible to make the same problem on my machine?

And, would you please make your project proceed without the migration cache code for a while?

Thanks,
Hirokazu Takahashi.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#37 lore raw
Quoted 11 lines · Hirokazu Takahashi
Quoted 5 lines

If I take out the migration cache patch, this "VM: killing ..." problem
goes away. So it has something to do specifically with the migration
cache code.

I've never seen the message though the migration cache code may have some bugs. May I ask you some questions about it?

- Which version of kernel did you use for it?

2.6.10. I pulled enough of the mm fixes (2 patches) so that the base migration patch from the hotplug tree would work on top of 2.6.10. AFAIK the same problem occurs on 2.6.11-mm2 which is where I started with the migration cache patch. But I admit I haven't tested it there recently.

Quoted 1 line

- Which migration cache code did you choose?

I'm using a version from Dec 8 I grabbed from an email from you to Marcello
titled: Subject: Re: migration cache, updated

Quoted 1 line

- How many nodes, CPUs and memory does your box have?

8 CPU, 4 Node Altix, but I really don't think that is significant.

Quoted 1 line

- What kind of applications were running on your box?

Machine was running single user. The only thing that was running was the test program that calls the page migration system call I wrote.

Quoted 1 line

- How often did this happened?

Every time.

Quoted 1 line

- Did this message appear right after starting the migration?

The pages all get migrated and then when the system call initiating all of this returns, the calling process gets killed. There is a printf following the system call that doesn't happen; the VM kill occurs first.

Quoted 1 line

Or it appeared some short while later?

Immediately on return.

Quoted 1 line

- How the target pages to be migrated were selected?

The system call interface specifes a virtual address range and pid. We scan through all pages in the vma specified by the address range (the range is required to be withing one vma). All resident pages in the range are pulled off of the lru list and added to the list to be passed in to try_to_migrate_pages().

Quoted 1 line

- How did you kick memory migration started?

Via the system call mentioned above.

Quoted 1 line

- Please show me /proc/meminfo when the problem happened.

Unfortunately, I don't have that data. There was lots of memory free, since I was running single user.

Quoted 1 line

- Is it possible to make the same problem on my machine?

I think so. I'd have to send you my system call code and test programs. Its not a lot of code on top of the existing page migration patch.

Quoted 3 lines

And, would you please make your project proceed without the migration cache code for a while?

I've already done that. :-)

Quoted 4 lines

Thanks,
Hirokazu Takahashi.

Signature · 12 lines
-----------------------------------------------
Ray Bryant
512-453-9679 (work)         512-507-7807 (cell)
[email protected]             [email protected]
The box said: "Requires Windows 98 or better",
	 so I installed Linux.
-----------------------------------------------
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#38 lore raw

Hi Ray,

Quoted 14 lines
Quoted 9 lines
Quoted 3 lines

If I take out the migration cache patch, this "VM: killing ..." problem
goes away. So it has something to do specifically with the migration
cache code.

I've never seen the message though the migration cache code may have some bugs. May I ask you some questions about it?

- Which version of kernel did you use for it?

2.6.10. I pulled enough of the mm fixes (2 patches) so that the base migration patch from the hotplug tree would work on top of 2.6.10. AFAIK the same problem occurs on 2.6.11-mm2 which is where I started with the migration cache patch. But I admit I haven't tested it there recently.

(snip)

Quoted 4 lines
Quoted 1 line

- Is it possible to make the same problem on my machine?

I think so. I'd have to send you my system call code and test programs. Its not a lot of code on top of the existing page migration patch.

Ok, would you post the code on the list?
I'll take a look at it and run on my box.

Quoted 4 lines
Quoted 2 lines

And, would you please make your project proceed without the migration cache code for a while?

I've already done that. :-)

Thanks,
Hirokazu Takahashi.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#39 lore raw

Hirokazu and Marcello,

I'll post my patches ASAP, but that may still be a day or so yet. There is a lot of development code here that the list may not be interested in. I wonder if it would be better just to send the two of you the code to test?

Signature · 14 lines
Best Regards,
Ray
-----------------------------------------------
                  Ray Bryant
512-453-9679 (work)         512-507-7807 (cell)
[email protected]             [email protected]
The box said: "Requires Windows 98 or better",
           so I installed Linux.
-----------------------------------------------
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#40 lore raw

Hi Ray,

Quoted 6 lines

Hirokazu and Marcello,

I'll post my patches ASAP, but that may still be a day or so yet. There is a lot of development code here that the list may not be interested in. I wonder if it would be better just to send the two of you the code to test?

Ok, if you don't want open it yet:)

Thanks,
Hirokazu Takahashi.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#43 lore raw

Hirokazu and Marcello,

Here's some more information on this problem I am having with the migration cache.

(The problem is that the test application is failing after it returns from the system call that migrated some of its address space from node 0 to node 3 on my test box. When the program touches the first page in the range that was migrated, the process gets killed because do_swap_page() returns VM_FAULT_OOM. The test works fine if I remove the migration cache patch.)

It looks like the page is flagged as being a migration pte, the page is found in the migration cache, but then the test

           "likely(pte_same(*page_table, orig_pte))"

succeeds. It's not obvious to me, at the moment, what this is supposed to be doing.

Here is the code segment from do_swap_page(), with the debug printout that was triggered:

again:

        if (pte_is_migration(orig_pte)) {
                page = lookup_migration_cache(entry.val);
                if (!page) {
                        spin_lock(&mm->page_table_lock);
                        page_table = pte_offset_map(pmd, address);
                        if (likely(pte_same(*page_table, orig_pte))) {
==========================>     DEBUG_VM_KILL(address);
                                ret = VM_FAULT_OOM;
                        }
                        else
                                ret = VM_FAULT_MINOR;
                        pte_unmap(page_table);
                        spin_unlock(&mm->page_table_lock);
                        goto out;
                }
Signature · 14 lines
Best Regards,
Ray
-----------------------------------------------
                  Ray Bryant
512-453-9679 (work)         512-507-7807 (cell)
[email protected]             [email protected]
The box said: "Requires Windows 98 or better",
           so I installed Linux.
-----------------------------------------------
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#44 lore raw
In reply to Ray Bryant

Hi Ray,

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines

(This message comes from ia64_do_page_fault() and appears to because handle_mm_fault() returned FAULT_OOM....)

I haven't looked into this further, but was wondering if perhaps one of you would understand why the migrate cache patch would fail in this way?

Quoted 8 lines
Quoted 3 lines

I can't think of anything right now - probably do_wp_page() is returning FAULT_OOM, can you confirm that?

No, it doesn't appear to be do_wp_page(). It looks like get_swap_page() returns FAULT_OOM followed by get_user_pages() returning FAULT_OOM. For the page that causes the VM to kill the process, there is no return from get_user_pages() that returns FAULT_OOM. Not sure yet what is going on here.

Is get_swap_page() typo of do_swap_page()?

How did you make sure do_swap_page() returned VM_FAULT_OOM? I feel do_wp_page() might have returned VM_FAULT_OOM.

Would you please insert printks into the code everywhere VM_FAULT_OOM is handled and let's see what's going on?

Thanks,
Hirokazu Takahashi.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#45 lore raw
In reply to Marcelo Tosatti

Hi Marcelo,

Very sorry for the delayed response.

Quoted 35 lines

Hi Hirokazu,

Quoted 32 lines · Hirokazu Takahashi

Hi Marcelo,

Quoted 10 lines
Quoted 6 lines
Quoted 1 line

Sorry for the delay, been busy with other things.

No problem. Everyone knows you're doing hard work!

Quoted 2 lines
Quoted 2 lines

Therefore, I made pages removed from the migration cache at the end of generic_migrate_page() if they remain in the cache.

OK, removing migration pages at end of generic_migrate_page() should avoid the leak - that part of your patch is fine to me!

Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

The another is a fork() related problem. If fork() has occurred during page migrationa, the previous work may not go well. pages may not be removed from the migration cache.

Can you please expand on that one? I assume it works fine because copy_page_range() duplicates the migration page reference (and the migration pte), meaning that on exit (zap_pte_range) the migration pages should be removed through migration_remove_entry().

Yes, that's true.

Quoted 1 line

I dont see the problem - please correct me.

However, once the page is moved into the migration cache, no one can make it swapped out. This problem may be solved by your approach described below.

Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

So I made the swapcode ignore pages in the migration cache. However, as you know this is just a workaround and not a correct way to fix it.

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

What this has to do with fork()? I can't understand.

fork() may leave some pages in the migration cache with my latest implementation, though the memory migration code tries to remove them from the migration cache by forcible pagefault in touch_unmapped_address().

Why are record_unmapped_address/touch_unmapped_address needed ?

There are two reasons.
1. Migrated pages should be mapped to the process space soon if they're mlocked. Or the pages might be swapped out as the swap code doesn't care about them if they aren't mapped.

2. Without this, migrated pages will consume entries of the migration cache until the process which the pages belong to has died.
And if they're kept in the migration cache, the pages cannot be swapped out even if free pages might become few, as you mentioned below.

Previously this is designed to not consume swap entries on real devices.

Quoted 4 lines

I started investigating the issue which migration pages couldnt be swapped out, but found out that migration pages are never left in the cache because touch_unmapped_address recreates the ptes removing the pages from the migration cache.

Yes. This is the one of the purpose.

Quoted 4 lines

That means we have no problem with migration cache pages left pinned (unswappable) in memory, which means it is fully functional AFAICT.

However, I thought the intent was to fault the pages on demand?

Yes, you're absolutely right.
If there is another better way, I'm pleased to replace it.

Quoted 2 lines

I even wrote this to be called at vmscan() time but touch_unmapped_address already has similar functionality at migration time.

Interesting.
It would work pretty good if fork() is invoked during memory migration. My touch_unmapped_address approach can't handle this case.

But I'm worried about two things.

- I wonder if mlocked pages can be handled correctly. What would happen if the page has been mlocked and it also belongs to the swap cache even though this case is very very rare?

- I'm not sure deriving anon_vma from the page is always correct while it isn't mapped to anywhere.

Quoted 59 lines

int try_to_faultin(struct page *page)

{
        struct anon_vma *anon_vma;
        struct vm_area_struct *vma;
        unsigned long address;
        int ret = 0;

restart:

        anon_vma = page_lock_anon_vma(page);
        if (!anon_vma)
                return ret;

        list_for_each_entry(vma, &anon_vma->head, anon_vma_node) {
                address = vma_address(page, vma);
                // handle if (address = -EFAULT) ? 
                if (!follow_and_check_present(vma->vm_mm, address))
                        continue;

                spin_unlock(&anon_vma->lock);
                switch (handle_mm_fault(vma->vm_mm, vma, address, 0)) {
                case VM_FAULT_MINOR:
                        goto restart;
                case VM_FAULT_MAJOR:
                        BUG();
                case VM_FAULT_SIGBUS:
                case VM_FAULT_OOM:
                        goto out_unlock;
                }
        }
        ret = 1;
        printk(KERN_ERR "faulted migration page in!\n");

out_unlock:

        spin_unlock(&anon_vma->lock);
        return ret;

}

Quoted 12 lines

However, touch_unmapped_address() doesn't know that the migration page has been duplicated.

Quoted 8 lines

Your patch is correct here also - we can't reclaim migration cache pages.

mm/vmscan.c renamed from mm/vmscan.c~migration_cache_marcelo5 +4 -0
+	if (PageMigration(page)) {
+		write_unlock_irq(&mapping->tree_lock);
+		goto keep_locked;
+	}
Quoted 8 lines
Quoted 5 lines

An enhancement would be to force pagefault of all pte's mapping to a migration cache page on shrink_list.

similar to rmap.c's try_to_unmap_anon() but intented to create the pte instead of unmapping it

If it works as we expect, this code can be called at the end of generic_migrate_page() I guess.

Quoted 6 lines

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>