Prevent LRU churing

There are some places to isolate lru page and I believe users of isolate_lru_page will be growing.
The purpose of them is each different so part of isolated pages should put back to LRU, again.

The problem is when we put back the page into LRU,
we lose LRU ordering and the page is inserted at head of LRU list. It makes unnecessary LRU churning so that vm can evict working set pages rather than idle pages.

This patch adds new filter mask when we isolate page in LRU. So, we don't isolate pages if we can't handle it. It could reduce LRU churning.

This patch shouldn't change old behavior.
It's just used by next patches.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 include/linux/swap.h |    3 ++-
 mm/compaction.c      |    2 +-
 mm/memcontrol.c      |    2 +-
 mm/vmscan.c          |   26 ++++++++++++++++++++------
 4 files changed, 24 insertions(+), 9 deletions(-)

include/linux/swap.h +0 -0

index 384eb5f..baef4ad 100644

Quoted 14 lines · On Wed, 27 Apr 2011 01:25:18 +0900 Minchan Kim <[email protected]> wrote:

There are some places to isolate lru page and I believe users of isolate_lru_page will be growing.
The purpose of them is each different so part of isolated pages should put back to LRU, again.

The problem is when we put back the page into LRU,
we lose LRU ordering and the page is inserted at head of LRU list. It makes unnecessary LRU churning so that vm can evict working set pages rather than idle pages.

This patch adds new filter mask when we isolate page in LRU. So, we don't isolate pages if we can't handle it. It could reduce LRU churning.

Quoted 8 lines

This patch shouldn't change old behavior.
It's just used by next patches.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Reviewed-by KAMEZAWA Hiroyuki <[email protected]>

nitpick below.

Quoted 7 lines

---
 include/linux/swap.h |    3 ++- mm/compaction.c      |    2 +- mm/memcontrol.c      |    2 +-
 mm/vmscan.c          |   26 ++++++++++++++++++++------

 4 files changed, 24 insertions(+), 9 deletions(-)

include/linux/swap.h extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem, · line 263

Quoted 11 lines

include/linux/swap.h +2 -1

index 384eb5f..baef4ad 100644
@@ -259,7 +259,8 @@ extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
                                               unsigned int swappiness,
                                               struct zone *zone,
                                               unsigned long *nr_scanned);
-extern int __isolate_lru_page(struct page *page, int mode, int file);
+extern int __isolate_lru_page(struct page *page, int mode, int file,
+                             int not_dirty, int not_mapped);

Hmm, which is better to use 4 binary args or a flag with bitmask ?

Thanks,
-Kame

There are some places to isolate lru page and I believe users of isolate_lru_page will be growing.
The purpose of them is each different so part of isolated pages should put back to LRU, again.

The problem is when we put back the page into LRU,
we lose LRU ordering and the page is inserted at head of LRU list. It makes unnecessary LRU churning so that vm can evict working set pages rather than idle pages.

This patch adds new filter mask when we isolate page in LRU. So, we don't isolate pages if we can't handle it. It could reduce LRU churning.

This patch shouldn't change old behavior.
It's just used by next patches.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 include/linux/swap.h |    3 ++- mm/compaction.c      |    2 +- mm/memcontrol.c      |    2 +-
 mm/vmscan.c          |   26 ++++++++++++++++++++------

 4 files changed, 24 insertions(+), 9 deletions(-)

include/linux/swap.h +2 -1

index 384eb5f..baef4ad 100644
@@ -259,7 +259,8 @@ extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
                                                unsigned int swappiness,
                                                struct zone *zone,
                                                unsigned long *nr_scanned);
-extern int __isolate_lru_page(struct page *page, int mode, int file);
+extern int __isolate_lru_page(struct page *page, int mode, int file,
+                               int not_dirty, int not_mapped);
  extern unsigned long shrink_all_memory(unsigned long nr_pages);
  extern int vm_swappiness;
  extern int remove_mapping(struct address_space *mapping, struct page *page);

mm/compaction.c +1 -1

index 021a296..dea32e3 100644
@@ -335,7 +335,7 @@ static unsigned long isolate_migratepages(struct zone *zone,
                }
 
                /* Try isolate the page */
-               if (__isolate_lru_page(page, ISOLATE_BOTH, 0) != 0)
+               if (__isolate_lru_page(page, ISOLATE_BOTH, 0, 0, 0) != 0)
                        continue;
 
                VM_BUG_ON(PageTransCompound(page));

mm/memcontrol.c +1 -1

index c2776f1..471e7fd 100644
@@ -1193,7 +1193,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
                        continue;
 
                scan++;
-               ret = __isolate_lru_page(page, mode, file);
+               ret = __isolate_lru_page(page, mode, file, 0, 0);
                switch (ret) {

               case 0:
                       list_move(&page->lru, dst);

mm/vmscan.c +5 -2

index b3a569f..71d2da9 100644
@@ -954,10 +954,13 @@ keep_lumpy:
  *
  * page:       page to consider
  * mode:       one of the LRU isolation modes defined above
- *
+ * file:       page be on a file LRU
+ * not_dirty:  page should be not dirty or not writeback
+ * not_mapped: page should be not mapped
  * returns 0 on success, -ve errno on failure.
  */
-int __isolate_lru_page(struct page *page, int mode, int file)
+int __isolate_lru_page(struct page *page, int mode, int file,
+                               int not_dirty, int not_mapped)
  {
        int ret = -EINVAL;
 

There are some places to isolate lru page and I believe users of isolate_lru_page will be growing.
The purpose of them is each different so part of isolated pages should put back to LRU, again.

The problem is when we put back the page into LRU,
we lose LRU ordering and the page is inserted at head of LRU list. It makes unnecessary LRU churning so that vm can evict working set pages rather than idle pages.

This patch adds new filter mask when we isolate page in LRU. So, we don't isolate pages if we can't handle it. It could reduce LRU churning.

This patch shouldn't change old behavior.
It's just used by next patches.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 include/linux/swap.h |    3 ++-
 mm/compaction.c      |    2 +-
 mm/memcontrol.c      |    2 +-
 mm/vmscan.c          |   26 ++++++++++++++++++++------
 4 files changed, 24 insertions(+), 9 deletions(-)

include/linux/swap.h +0 -0

index 384eb5f..baef4ad 100644

include/linux/swap.h +0 -0

 extern unsigned long shrink_all_memory(unsigned long nr_pages);
 extern int vm_swappiness;
 extern int remove_mapping(struct address_space *mapping, struct page *page);

mm/compaction.c +1 -1

index 021a296..dea32e3 100644
@@ -335,7 +335,7 @@ static unsigned long isolate_migratepages(struct zone *zone,
 		}
 
 		/* Try isolate the page */
-		if (__isolate_lru_page(page, ISOLATE_BOTH, 0) != 0)
+		if (__isolate_lru_page(page, ISOLATE_BOTH, 0, 0, 0) != 0)
 			continue;
 
 		VM_BUG_ON(PageTransCompound(page));

mm/memcontrol.c +1 -1

index c2776f1..471e7fd 100644
@@ -1193,7 +1193,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
 			continue;
 
 		scan++;
-		ret = __isolate_lru_page(page, mode, file);
+		ret = __isolate_lru_page(page, mode, file, 0, 0);
 		switch (ret) {
 		case 0:
 			list_move(&page->lru, dst);

mm/vmscan.c +20 -6

index b3a569f..71d2da9 100644
@@ -954,10 +954,13 @@ keep_lumpy:
  *
  * page:	page to consider
  * mode:	one of the LRU isolation modes defined above
- *
+ * file:	page be on a file LRU
+ * not_dirty:	page should be not dirty or not writeback
+ * not_mapped:	page should be not mapped
  * returns 0 on success, -ve errno on failure.
  */
-int __isolate_lru_page(struct page *page, int mode, int file)
+int __isolate_lru_page(struct page *page, int mode, int file,
+				int not_dirty, int not_mapped)
 {
 	int ret = -EINVAL;
 

@@ -976,6 +979,12 @@ int __isolate_lru_page(struct page *page, int mode, int file)
 	if (mode != ISOLATE_BOTH && page_is_file_cache(page) != file)
 		return ret;
 
+	if (not_dirty)
+		if (PageDirty(page) || PageWriteback(page))
+			return ret;
+	if (not_mapped)
+		if (page_mapped(page))
+			return ret;
 	/*
 	 * When this function is being called for lumpy reclaim, we
 	 * initially look into all LRU pages, active, inactive and

@@ -1016,12 +1025,15 @@ int __isolate_lru_page(struct page *page, int mode, int file)
  * @order:	The caller's attempted allocation order
  * @mode:	One of the LRU isolation modes
  * @file:	True [1] if isolating file [!anon] pages
+ * @not_dirty:	True [1] if isolating file [!dirty] pages
+ * @not_mapped:	True [1] if isolating file [!mapped] pages
  *
  * returns how many pages were moved onto *@dst.
  */
 static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 		struct list_head *src, struct list_head *dst,
-		unsigned long *scanned, int order, int mode, int file)
+		unsigned long *scanned, int order, int mode, int file,
+		int not_dirty, int not_mapped)
 {
 	unsigned long nr_taken = 0;
 	unsigned long nr_lumpy_taken = 0;

@@ -1041,7 +1053,8 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 
 		VM_BUG_ON(!PageLRU(page));
 
-		switch (__isolate_lru_page(page, mode, file)) {
+		switch (__isolate_lru_page(page, mode, file,
+					not_dirty, not_mapped)) {
 		case 0:
 			list_move(&page->lru, dst);
 			mem_cgroup_del_lru(page);

@@ -1100,7 +1113,8 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 			    !PageSwapCache(cursor_page))
 				break;
 
-			if (__isolate_lru_page(cursor_page, mode, file) == 0) {
+			if (__isolate_lru_page(cursor_page, mode, file,
+					not_dirty, not_mapped) == 0) {
 				list_move(&cursor_page->lru, dst);
 				mem_cgroup_del_lru(cursor_page);
 				nr_taken += hpage_nr_pages(page);

@@ -1143,7 +1157,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
 	if (file)
 		lru += LRU_FILE;
 	return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
-								mode, file);
+					mode, file, 0, 0);
 }
 
 /*

Signature · 1 line

1.7.1

In async mode, compaction doesn't migrate dirty or writeback pages. So, it's meaningless to pick the page and re-add it to lru list.

Of course, when we isolate the page in compaction, the page might be dirty or writeback but when we try to migrate the page, the page would be not dirty, writeback. So it could be migrated. But it's very unlikely as isolate and migration cycle is much faster than writeout.

So, this patch helps cpu and prevent unnecessary LRU churning.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

In async mode, compaction doesn't migrate dirty or writeback pages. So, it's meaningless to pick the page and re-add it to lru list.

Of course, when we isolate the page in compaction, the page might be dirty or writeback but when we try to migrate the page, the page would be not dirty, writeback. So it could be migrated. But it's very unlikely as isolate and migration cycle is much faster than writeout.

So, this patch helps cpu and prevent unnecessary LRU churning.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 mm/compaction.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

mm/compaction.c +0 -0

index dea32e3..9f80b5a 100644

Quoted 25 lines · Minchan Kim

In async mode, compaction doesn't migrate dirty or writeback pages. So, it's meaningless to pick the page and re-add it to lru list.

Of course, when we isolate the page in compaction, the page might be dirty or writeback but when we try to migrate the page, the page would be not dirty, writeback. So it could be migrated. But it's very unlikely as isolate and migration cycle is much faster than writeout.

So, this patch helps cpu and prevent unnecessary LRU churning.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 mm/compaction.c |    2 +-

 1 files changed, 1 insertions(+), 1 deletions(-)

mm/compaction.c +0 -0

index dea32e3..9f80b5a 100644

mm/compaction.c static unsigned long isolate_migratepages(struct zone *zone, · line 339

Quoted 6 lines

mm/compaction.c +1 -1

@@ -335,7 +335,7 @@ static unsigned long isolate_migratepages(struct zone *zone,
               }
 
               /* Try isolate the page */
-             if (__isolate_lru_page(page, ISOLATE_BOTH, 0, 0, 0) != 0)
+             if (__isolate_lru_page(page, ISOLATE_BOTH, 0, !cc->sync, 0) != 0)
                       continue;

With the suggested flags argument from 1/8, this would look like:

       flags = ISOLATE_BOTH;
       if (!cc->sync)
               flags |= ISOLATE_CLEAN;

?

Anyway, nice change indeed!

Quoted 6 lines

With the suggested flags argument from 1/8, this would look like:

       flags = ISOLATE_BOTH;
       if (!cc->sync)
               flags |= ISOLATE_CLEAN;

Quoted 1 line

?

Yes. I will change it.

Quoted 2 lines

Anyway, nice change indeed!

Thanks!

In async mode, compaction doesn't migrate dirty or writeback pages. So, it's meaningless to pick the page and re-add it to lru list.

Of course, when we isolate the page in compaction, the page might be dirty or writeback but when we try to migrate the page, the page would be not dirty, writeback. So it could be migrated. But it's very unlikely as isolate and migration cycle is much faster than writeout.

So, this patch helps cpu and prevent unnecessary LRU churning.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

In some __zone_reclaim case, we don't want to shrink mapped page. Nonetheless, we have isolated mapped page and re-add it into LRU's head. It's unnecessary CPU overhead and makes LRU churning.

Of course, when we isolate the page, the page might be mapped but when we try to migrate the page, the page would be not mapped. So it could be migrated. But race is rare and although it happens, it's no big deal.

Cc Christoph Lameter <[email protected]>

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 mm/vmscan.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

mm/vmscan.c +6 -5

index 71d2da9..e8d6190 100644
@@ -1147,7 +1147,8 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 
 static unsigned long isolate_pages_global(unsigned long nr,
 					struct list_head *dst,
-					unsigned long *scanned, int order,
+					unsigned long *scanned,
+					struct scan_control *sc,
 					int mode, struct zone *z,
 					int active, int file)
 {

@@ -1156,8 +1157,8 @@ static unsigned long isolate_pages_global(unsigned long nr,
 		lru += LRU_ACTIVE;
 	if (file)
 		lru += LRU_FILE;
-	return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
-					mode, file, 0, 0);
+	return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, sc->order,
+					mode, file, 0, !sc->may_unmap);
 }
 
 /*

@@ -1407,7 +1408,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct zone *zone,
 
 	if (scanning_global_lru(sc)) {
 		nr_taken = isolate_pages_global(nr_to_scan,
-			&page_list, &nr_scanned, sc->order,
+			&page_list, &nr_scanned, sc,
 			sc->reclaim_mode & RECLAIM_MODE_LUMPYRECLAIM ?
 					ISOLATE_BOTH : ISOLATE_INACTIVE,
 			zone, 0, file);

@@ -1531,7 +1532,7 @@ static void shrink_active_list(unsigned long nr_pages, struct zone *zone,
 	spin_lock_irq(&zone->lru_lock);
 	if (scanning_global_lru(sc)) {
 		nr_taken = isolate_pages_global(nr_pages, &l_hold,
-						&pgscanned, sc->order,
+						&pgscanned, sc,
 						ISOLATE_ACTIVE, zone,
 						1, file);
 		zone->pages_scanned += pgscanned;

Signature · 7 lines

1.7.1

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

Quoted 9 lines · On Wed, 27 Apr 2011 01:25:20 +0900 Minchan Kim <[email protected]> wrote:

In some __zone_reclaim case, we don't want to shrink mapped page. Nonetheless, we have isolated mapped page and re-add it into LRU's head. It's unnecessary CPU overhead and makes LRU churning.

Of course, when we isolate the page, the page might be mapped but when we try to migrate the page, the page would be not mapped. So it could be migrated. But race is rare and although it happens, it's no big deal.

Quoted 6 lines

Cc Christoph Lameter <[email protected]>

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Hmm, it seems mm/memcontrol.c::mem_cgroup_isolate_pages() should be updated, too.

But it's okay you start from global LRU.

Reviewed-by KAMEZAWA Hiroyuki <[email protected]>

Quoted 9 lines · On Wed, 27 Apr 2011 01:25:20 +0900 Minchan Kim <[email protected]> wrote:

In some __zone_reclaim case, we don't want to shrink mapped page. Nonetheless, we have isolated mapped page and re-add it into LRU's head. It's unnecessary CPU overhead and makes LRU churning.

Of course, when we isolate the page, the page might be mapped but when we try to migrate the page, the page would be not mapped. So it could be migrated. But race is rare and although it happens, it's no big deal.

Quoted 6 lines

Cc Christoph Lameter <[email protected]>

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Hmm, it seems mm/memcontrol.c::mem_cgroup_isolate_pages() should be updated, too.

Quoted 20 lines · KAMEZAWA Hiroyuki

Quoted 15 lines · On Wed, 27 Apr 2011 01:25:20 +0900 Minchan Kim <[email protected]> wrote:

In some __zone_reclaim case, we don't want to shrink mapped page. Nonetheless, we have isolated mapped page and re-add it into LRU's head. It's unnecessary CPU overhead and makes LRU churning.

Of course, when we isolate the page, the page might be mapped but when we try to migrate the page, the page would be not mapped. So it could be migrated. But race is rare and although it happens, it's no big deal.

Cc Christoph Lameter <[email protected]>

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 1 line

Hmm, it seems mm/memcontrol.c::mem_cgroup_isolate_pages() should be updated, too.

memcg reclaim always does sc->may_unmap = 1. What is there to communicate to mem_cgroup_isolate_pages?

Quoted 21 lines · On Thu, 28 Apr 2011 10:54:32 +0200 Johannes Weiner <[email protected]> wrote:

Quoted 20 lines · KAMEZAWA Hiroyuki

Quoted 15 lines · On Wed, 27 Apr 2011 01:25:20 +0900 Minchan Kim <[email protected]> wrote:

In some __zone_reclaim case, we don't want to shrink mapped page. Nonetheless, we have isolated mapped page and re-add it into LRU's head. It's unnecessary CPU overhead and makes LRU churning.

Of course, when we isolate the page, the page might be mapped but when we try to migrate the page, the page would be not mapped. So it could be migrated. But race is rare and although it happens, it's no big deal.

Cc Christoph Lameter <[email protected]>

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 5 lines

Quoted 1 line

Hmm, it seems mm/memcontrol.c::mem_cgroup_isolate_pages() should be updated, too.

memcg reclaim always does sc->may_unmap = 1. What is there to communicate to mem_cgroup_isolate_pages?

Hmm, maybe you're right and nothing to do until memcg need to support soft limit in zone reclaim mode. I hope no more users.

In some __zone_reclaim case, we don't want to shrink mapped page. Nonetheless, we have isolated mapped page and re-add it into LRU's head. It's unnecessary CPU overhead and makes LRU churning.

Of course, when we isolate the page, the page might be mapped but when we try to migrate the page, the page would be not mapped. So it could be migrated. But race is rare and although it happens, it's no big deal.

Cc Christoph Lameter <[email protected]>

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

index 71d2da9..e8d6190 100644

mm/vmscan.c +2 -1

@@ -1147,7 +1147,8 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 
 static unsigned long isolate_pages_global(unsigned long nr,
 					struct list_head *dst,
-					unsigned long *scanned, int order,
+					unsigned long *scanned,
+					struct scan_control *sc,
 					int mode, struct zone *z,
 					int active, int file)
 {

mm/vmscan.c +2 -2

 /*

@@ -1407,7 +1408,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct zone *zone,
 
 	if (scanning_global_lru(sc)) {
 		nr_taken = isolate_pages_global(nr_to_scan,
-			&page_list, &nr_scanned, sc->order,
+			&page_list, &nr_scanned, sc,
 			sc->reclaim_mode & RECLAIM_MODE_LUMPYRECLAIM ?
 					ISOLATE_BOTH : ISOLATE_INACTIVE,
 			zone, 0, file);

@@ -1531,7 +1532,7 @@ static void shrink_active_list(unsigned long nr_pages, struct zone *zone,
 	spin_lock_irq(&zone->lru_lock);
 	if (scanning_global_lru(sc)) {
 		nr_taken = isolate_pages_global(nr_pages, &l_hold,
-						&pgscanned, sc->order,
+						&pgscanned, sc,
 						ISOLATE_ACTIVE, zone,
 						1, file);
 		zone->pages_scanned += pgscanned;

Signature · 1 line

1.7.1

Quoted 33 lines · Minchan Kim

In some __zone_reclaim case, we don't want to shrink mapped page. Nonetheless, we have isolated mapped page and re-add it into LRU's head. It's unnecessary CPU overhead and makes LRU churning.

Of course, when we isolate the page, the page might be mapped but when we try to migrate the page, the page would be not mapped. So it could be migrated. But race is rare and although it happens, it's no big deal.

Cc Christoph Lameter <[email protected]>

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

index 71d2da9..e8d6190 100644

mm/vmscan.c +2 -1

@@ -1147,7 +1147,8 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 
  static unsigned long isolate_pages_global(unsigned long nr,
                                       struct list_head *dst,
-                                     unsigned long *scanned, int order,
+                                     unsigned long *scanned,
+                                     struct scan_control *sc,
                                       int mode, struct zone *z,
                                       int active, int file)
  {

@@ -1156,8 +1157,8 @@ static unsigned long isolate_pages_global(unsigned long nr,
               lru += LRU_ACTIVE;
       if (file)
               lru += LRU_FILE;

mm/vmscan.c static unsigned long isolate_pages_global(unsigned long nr, · line 1163

Quoted 6 lines

mm/vmscan.c +2 -2

-     return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
-                                     mode, file, 0, 0);
+     return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, sc->order,
+                                     mode, file, 0, !sc->may_unmap);
  }
 

Why not take may_writepage into account for dirty pages?

Commonly, putback_lru_page is used with isolated_lru_page. The isolated_lru_page picks the page in middle of LRU and putback_lru_page insert the lru in head of LRU.
It means it could make LRU churning so we have to be very careful. Let's clear description of putback_lru_page.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 11 lines · On Wed, 27 Apr 2011 01:25:21 +0900 Minchan Kim <[email protected]> wrote:

Commonly, putback_lru_page is used with isolated_lru_page. The isolated_lru_page picks the page in middle of LRU and putback_lru_page insert the lru in head of LRU.
It means it could make LRU churning so we have to be very careful. Let's clear description of putback_lru_page.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

seems good...

Reviewed-by KAMEZAWA Hiroyuki <[email protected]>

But is there consensus which side of LRU is tail? head?

I always need to revisit codes when I see a word head/tail....

Quoted 15 lines · KAMEZAWA Hiroyuki

Quoted 11 lines · On Wed, 27 Apr 2011 01:25:21 +0900 Minchan Kim <[email protected]> wrote:

Commonly, putback_lru_page is used with isolated_lru_page. The isolated_lru_page picks the page in middle of LRU and putback_lru_page insert the lru in head of LRU.
It means it could make LRU churning so we have to be very careful. Let's clear description of putback_lru_page.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 4 lines

seems good...

Reviewed-by KAMEZAWA Hiroyuki <[email protected]>

But is there consensus which side of LRU is tail? head?

I don't know. I used to think it's head.
If other guys raise a concern as well, let's talk about it. :) Thanks

Quoted 22 lines

Quoted 19 lines · KAMEZAWA Hiroyuki

Quoted 11 lines · On Wed, 27 Apr 2011 01:25:21 +0900 Minchan Kim <[email protected]> wrote:

Commonly, putback_lru_page is used with isolated_lru_page. The isolated_lru_page picks the page in middle of LRU and putback_lru_page insert the lru in head of LRU.
It means it could make LRU churning so we have to be very careful. Let's clear description of putback_lru_page.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

seems good...

Reviewed-by KAMEZAWA Hiroyuki <[email protected]>

But is there consensus which side of LRU is tail? head?

Quoted 3 lines

I don't know. I used to think it's head.
If other guys raise a concern as well, let's talk about it. :) Thanks

I suppose we add new pages to the head of the LRU and reclaim old pages from the tail.

Acked-by Johannes Weiner <[email protected]>

Quoted 27 lines · Minchan Kim

Quoted 25 lines

Quoted 15 lines · KAMEZAWA Hiroyuki

Quoted 11 lines · On Wed, 27 Apr 2011 01:25:21 +0900 Minchan Kim <[email protected]> wrote:

Commonly, putback_lru_page is used with isolated_lru_page. The isolated_lru_page picks the page in middle of LRU and putback_lru_page insert the lru in head of LRU.
It means it could make LRU churning so we have to be very careful. Let's clear description of putback_lru_page.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 4 lines

seems good...

Reviewed-by KAMEZAWA Hiroyuki <[email protected]>

But is there consensus which side of LRU is tail? head?

I don't know. I used to think it's head.
If other guys raise a concern as well, let's talk about it. :) Thanks

Quoted 4 lines

I suppose we add new pages to the head of the LRU and reclaim old pages from the tail.

Acked-by Johannes Weiner <[email protected]>

So, It would be better if isolate_lru_page() also have "LRU tail blah blah blah" comments.

anyway,

Reviewed-by KOSAKI Motohiro <[email protected]>

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 mm/compaction.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

mm/compaction.c +2 -2

index 9f80b5a..653b02b 100644
@@ -219,8 +219,8 @@ static void acct_isolated(struct zone *zone, struct compact_control *cc)
 		count[lru]++;
 	}
 
-	cc->nr_anon = count[LRU_ACTIVE_ANON] + count[LRU_INACTIVE_ANON];
-	cc->nr_file = count[LRU_ACTIVE_FILE] + count[LRU_INACTIVE_FILE];
+	cc->nr_anon = count[LRU_INACTIVE_ANON];
+	cc->nr_file = count[LRU_INACTIVE_FILE];
 	__mod_zone_page_state(zone, NR_ISOLATED_ANON, cc->nr_anon);
 	__mod_zone_page_state(zone, NR_ISOLATED_FILE, cc->nr_file);
 }

Signature · 7 lines

1.7.1

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

Quoted 4 lines · On Wed, 27 Apr 2011 01:25:22 +0900 Minchan Kim <[email protected]> wrote:

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Quoted 5 lines

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

I think some comments are necessary to explain why INACTIVE only.

Reviewed-by KAMEZAWA Hiroyuki <[email protected]>

Quoted 13 lines · KAMEZAWA Hiroyuki

Quoted 9 lines · On Wed, 27 Apr 2011 01:25:22 +0900 Minchan Kim <[email protected]> wrote:

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 1 line

I think some comments are necessary to explain why INACTIVE only.

As alternative, I can change page_lru_base_type with page_lru. It's not hot path so I will do.

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 mm/compaction.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

mm/compaction.c +0 -0

index 9f80b5a..653b02b 100644

Quoted 21 lines · Minchan Kim

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 mm/compaction.c |    4 ++--

 1 files changed, 2 insertions(+), 2 deletions(-)

mm/compaction.c +0 -0

index 9f80b5a..653b02b 100644
@@ -219,8 +219,8 @@ static void acct_isolated(struct zone *zone, struct compact_control *cc)
               count[lru]++;
       }
 

mm/compaction.c static void acct_isolated(struct zone *zone, struct compact_control *cc) · line 226

Quoted 7 lines

mm/compaction.c +2 -2

-     cc->nr_anon = count[LRU_ACTIVE_ANON] + count[LRU_INACTIVE_ANON];
-     cc->nr_file = count[LRU_ACTIVE_FILE] + count[LRU_INACTIVE_FILE];
+     cc->nr_anon = count[LRU_INACTIVE_ANON];
+     cc->nr_file = count[LRU_INACTIVE_FILE];
       __mod_zone_page_state(zone, NR_ISOLATED_ANON, cc->nr_anon);
       __mod_zone_page_state(zone, NR_ISOLATED_FILE, cc->nr_file);
  }

I don't see anything using cc->nr_anon and cc->nr_file outside this code.  Would it make sense to remove them completely?

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 9 lines · Minchan Kim

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

hmm, isolate_migratepages() is doing a linear scan of PFNs and is calling __isolate_lru_page(..ISOLATE_BOTH..). Using page_lru_base_type happens to work because we're only interested in the number of isolated pages and your patch still covers that. Using page_lru might be more accurate in terms of accountancy but does not seem necessary.

Adding a comment explaining why we account for it as inactive and why that's ok would be nice although I admit this is something I should have done when acct_isolated() was introduced.

Quoted 11 lines · Minchan Kim

Quoted 9 lines

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 5 lines

hmm, isolate_migratepages() is doing a linear scan of PFNs and is calling __isolate_lru_page(..ISOLATE_BOTH..). Using page_lru_base_type happens to work because we're only interested in the number of isolated pages and your patch still covers that. Using page_lru might be more accurate in terms of accountancy but does not seem necessary.

True.

Quoted 4 lines

Adding a comment explaining why we account for it as inactive and why that's ok would be nice although I admit this is something I should have done when acct_isolated() was introduced.

When Kame pointed out comment, I wanted to avoid unnecessary comment so decided changing it with page_lru although it adds overhead a little bit. But Hannes, you and maybe Kame don't want it. I don't mind adding comment.
Okay. fix it in next version.

Quoted 25 lines · Mel Gorman

Quoted 11 lines · Minchan Kim

Quoted 9 lines

acct_isolated of compaction uses page_lru_base_type which returns only base type of LRU list so it never returns LRU_ACTIVE_ANON or LRU_ACTIVE_FILE. So it's pointless to add lru[LRU_ACTIVE_[ANON|FILE]] to get sum.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

Quoted 5 lines

hmm, isolate_migratepages() is doing a linear scan of PFNs and is calling __isolate_lru_page(..ISOLATE_BOTH..). Using page_lru_base_type happens to work because we're only interested in the number of isolated pages and your patch still covers that. Using page_lru might be more accurate in terms of accountancy but does not seem necessary.

True.

Quoted 4 lines

Adding a comment explaining why we account for it as inactive and why that's ok would be nice although I admit this is something I should have done when acct_isolated() was introduced.

Quoted 5 lines

When Kame pointed out comment, I wanted to avoid unnecessary comment so decided changing it with page_lru although it adds overhead a little bit. But Hannes, you and maybe Kame don't want it. I don't mind adding comment.
Okay. fix it in next version.

Or

       unsigned int count[2];

       list_for_each_entry(page, &cc->migratepages, lru) { count[page_is_file_cache(page)]++; }

is also clear to me.

This patch defines new APIs to putback the page into previous position of LRU. The idea is simple.

When we try to putback the page into lru list and if friends(prev, next) of the pages still is nearest neighbor, we can insert isolated page into prev's next instead of head of LRU list. So it keeps LRU history without losing the LRU information.

Before :
       LRU POV : H - P1 - P2 - P3 - P4 -T

Isolate P3 :
       LRU POV : H - P1 - P2 - P4 - T

Putback P3 :
       if (P2->next == P4)
               putback(P3, P2);
       So,
       LRU POV : H - P1 - P2 - P3 - P4 -T

For implement, we defines new structure pages_lru which remebers both lru friend pages of isolated one and handling functions.

But this approach has a problem on contiguous pages.
In this case, my idea can not work since friend pages are isolated, too. It means prev_page->next == next_page always is false and both pages are not LRU any more at that time. It's pointed out by Rik at LSF/MM summit. So for solving the problem, I can change the idea.
I think we don't need both friend(prev, next) pages relation but just consider either prev or next page that it is still same LRU. Worset case in this approach, prev or next page is free and allocate new so it's in head of LRU and our isolated page is located on next of head. But it's almost same situation with current problem. So it doesn't make worse than now and it would be rare. But in this version, I implement based on idea discussed at LSF/MM. If my new idea makes sense, I will change it.

Any comment?

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 include/linux/migrate.h  |    2 +
 include/linux/mm_types.h |    7 ++++
 include/linux/swap.h     |    4 ++-
 mm/compaction.c          |    3 +-
 mm/internal.h            |    2 +
 mm/memcontrol.c          |    2 +-
 mm/migrate.c             |   36 +++++++++++++++++++++
 mm/swap.c                |    2 +-
 mm/vmscan.c              |   79 +++++++++++++++++++++++++++++++++++++++++++--

 9 files changed, 129 insertions(+), 8 deletions(-)

include/linux/migrate.h +2 -0

index e39aeec..3aa5ab6 100644
@@ -9,6 +9,7 @@ typedef struct page *new_page_t(struct page *, unsigned long private, int **);
  #ifdef CONFIG_MIGRATION
  #define PAGE_MIGRATION 1
 
+extern void putback_pages_lru(struct list_head *l);
  extern void putback_lru_pages(struct list_head *l);
  extern int migrate_page(struct address_space *,
                        struct page *, struct page *);

@@ -33,6 +34,7 @@ extern int migrate_huge_page_move_mapping(struct address_space *mapping,
  #else
  #define PAGE_MIGRATION 0
 
+static inline void putback_pages_lru(struct list_head *l) {}
  static inline void putback_lru_pages(struct list_head *l) {}
  static inline int migrate_pages(struct list_head *l, new_page_t x,
                unsigned long private, bool offlining,

include/linux/mm_types.h +7 -0

index ca01ab2..35e80fb 100644
@@ -102,6 +102,13 @@ struct page {
  #endif
  };
 
+/* This structure is used for keeping LRU ordering of isolated page */
+struct pages_lru {
+        struct page *page;      /* isolated page */
+        struct page *prev_page; /* previous page of isolate page as LRU order */
+        struct page *next_page; /* next page of isolate page as LRU order */
+        struct list_head lru;
+};
  /*
  * A region containing a mapping of a non-memory backed file under NOMMU
  * conditions.  These are held in a global tree and are pinned by the VMAs that

include/linux/swap.h +3 -1

index baef4ad..4ad0a0c 100644
@@ -227,6 +227,8 @@ extern void rotate_reclaimable_page(struct page *page);
  extern void deactivate_page(struct page *page);
  extern void swap_setup(void);
 
+extern void update_page_reclaim_stat(struct zone *zone, struct page *page,
+                                    int file, int rotated);
  extern void add_page_to_unevictable_list(struct page *page);
 
  /**

@@ -260,7 +262,7 @@ extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
                                                struct zone *zone,
                                                unsigned long *nr_scanned);
  extern int __isolate_lru_page(struct page *page, int mode, int file,
-                               int not_dirty, int not_mapped);
+               int not_dirty, int not_mapped, struct pages_lru *pages_lru);
  extern unsigned long shrink_all_memory(unsigned long nr_pages);
  extern int vm_swappiness;
  extern int remove_mapping(struct address_space *mapping, struct page *page);

mm/compaction.c +2 -1

index 653b02b..c453000 100644
@@ -335,7 +335,8 @@ static unsigned long isolate_migratepages(struct zone *zone,
                }
 
                /* Try isolate the page */
-               if (__isolate_lru_page(page, ISOLATE_BOTH, 0, !cc->sync, 0) != 0)
+               if (__isolate_lru_page(page, ISOLATE_BOTH, 0,
+                                       !cc->sync, 0, NULL) != 0)
                        continue;
 
                VM_BUG_ON(PageTransCompound(page));

mm/internal.h +2 -0

index d071d38..3c8182c 100644
@@ -43,6 +43,8 @@ extern unsigned long highest_memmap_pfn;
  * in mm/vmscan.c:
  */
  extern int isolate_lru_page(struct page *page);
+extern bool keep_lru_order(struct pages_lru *pages_lru);
+extern void putback_page_to_lru(struct page *page, struct list_head *head);
  extern void putback_lru_page(struct page *page);
 
  /*

mm/memcontrol.c +1 -1

index 471e7fd..92a9046 100644
@@ -1193,7 +1193,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
                        continue;
 
                scan++;
-               ret = __isolate_lru_page(page, mode, file, 0, 0);
+               ret = __isolate_lru_page(page, mode, file, 0, 0, NULL);
                switch (ret) {

               case 0:
                       list_move(&page->lru, dst);

mm/migrate.c +36 -0

index 819d233..9cfb63b 100644
@@ -85,6 +85,42 @@ void putback_lru_pages(struct list_head *l)
  }
 
  /*
+ * This function is almost same iwth putback_lru_pages.
+ * The difference is that function receives struct pages_lru list
+ * and if possible, we add pages into original position of LRU
+ * instead of LRU's head.
+ */
+void putback_pages_lru(struct list_head *l)
+{
+        struct pages_lru *isolated_page;
+        struct pages_lru *isolated_page2;
+        struct page *page;
+
+        list_for_each_entry_safe(isolated_page, isolated_page2, l, lru) {
+                struct zone *zone;
+                page = isolated_page->page;
+                list_del(&isolated_page->lru);
+
+                dec_zone_page_state(page, NR_ISOLATED_ANON +
+                                page_is_file_cache(page));
+
+                zone = page_zone(page);
+                spin_lock_irq(&zone->lru_lock);
+                if (keep_lru_order(isolated_page)) {
+                        putback_page_to_lru(page, &isolated_page->prev_page->lru);
+                        spin_unlock_irq(&zone->lru_lock);
+                }
+                else {
+                        spin_unlock_irq(&zone->lru_lock);
+                        putback_lru_page(page);
+                }
+
+                kfree(isolated_page);
+        }
+}
+
+
+/*
  * Restore a potential migration pte to a working pte entry
  */
  static int remove_migration_pte(struct page *new, struct vm_area_struct *vma,

mm/swap.c +1 -1

index a83ec5a..0cb15b7 100644
@@ -252,7 +252,7 @@ void rotate_reclaimable_page(struct page *page)
        }
  }
 
-static void update_page_reclaim_stat(struct zone *zone, struct page *page,
+void update_page_reclaim_stat(struct zone *zone, struct page *page,
                                     int file, int rotated)
  {
        struct zone_reclaim_stat *reclaim_stat = &zone->reclaim_stat;

mm/vmscan.c +57 -2

index 5196f0c..06a7c9b 100644
@@ -550,6 +550,58 @@ int remove_mapping(struct address_space *mapping, struct page *page)
        return 0;
  }
 
+/* zone->lru_lock must be hold */
+bool keep_lru_order(struct pages_lru *pages_lru)
+{
+        bool ret = false;
+        struct page *prev, *next;
+
+        if (!pages_lru->prev_page)
+                return ret;
+
+        prev = pages_lru->prev_page;
+        next = pages_lru->next_page;
+
+        if (!PageLRU(prev) || !PageLRU(next))
+                return ret;
+
+        if (prev->lru.next == &next->lru)
+                ret = true;
+
+       if (unlikely(PageUnevictable(prev)))
+               ret = false;
+
+        return ret;
+}
+
+/**
+ * putback_page_to_lru - put isolated @page onto @head
+ * @page: page to be put back to appropriate lru list
+ * @head: lru position to be put back
+ *
+ * Insert previously isolated @page to appropriate position of lru list
+ * zone->lru_lock must be hold.
+ */
+void putback_page_to_lru(struct page *page, struct list_head *head)
+{
+        int lru, active, file;
+        struct zone *zone = page_zone(page);
+        struct page *prev_page = container_of(head, struct page, lru);
+
+        lru = page_lru(prev_page);
+        active = is_active_lru(lru);
+        file = is_file_lru(lru);
+
+        if (active)
+                SetPageActive(page);
+       else
+               ClearPageActive(page);
+
+        update_page_reclaim_stat(zone, page, file, active);
+        SetPageLRU(page);
+        __add_page_to_lru_list(zone, page, lru, head);
+}
+
  /**
  * putback_lru_page - put previously isolated page onto appropriate LRU list's head
  * @page: page to be put back to appropriate lru list

@@ -959,8 +1011,8 @@ keep_lumpy:
  * not_mapped: page should be not mapped
  * returns 0 on success, -ve errno on failure.
  */
-int __isolate_lru_page(struct page *page, int mode, int file,
-                               int not_dirty, int not_mapped)
+int __isolate_lru_page(struct page *page, int mode, int file, int not_dirty,
+                               int not_mapped, struct pages_lru *pages_lru)
  {
        int ret = -EINVAL;
 

@@ -996,12 +1048,31 @@ int __isolate_lru_page(struct page *page, int mode, int file,
        ret = -EBUSY;
 
        if (likely(get_page_unless_zero(page))) {
+               struct zone *zone = page_zone(page);
+               enum lru_list l = page_lru(page);
                /*
                 * Be careful not to clear PageLRU until after we're
                 * sure the page is not being freed elsewhere -- the
                 * page release code relies on it.
                 */
                ClearPageLRU(page);
+

This patch defines new APIs to putback the page into previous position of LRU. The idea is simple.

When we try to putback the page into lru list and if friends(prev, next) of the pages still is nearest neighbor, we can insert isolated page into prev's next instead of head of LRU list. So it keeps LRU history without losing the LRU information.

Before :

	LRU POV : H - P1 - P2 - P3 - P4 -T

Isolate P3 :

	LRU POV : H - P1 - P2 - P4 - T

Putback P3 :

	if (P2->next == P4)
		putback(P3, P2);
	So,
	LRU POV : H - P1 - P2 - P3 - P4 -T

For implement, we defines new structure pages_lru which remebers both lru friend pages of isolated one and handling functions.

But this approach has a problem on contiguous pages.
In this case, my idea can not work since friend pages are isolated, too. It means prev_page->next == next_page always is false and both pages are not LRU any more at that time. It's pointed out by Rik at LSF/MM summit. So for solving the problem, I can change the idea.
I think we don't need both friend(prev, next) pages relation but just consider either prev or next page that it is still same LRU. Worset case in this approach, prev or next page is free and allocate new so it's in head of LRU and our isolated page is located on next of head. But it's almost same situation with current problem. So it doesn't make worse than now and it would be rare. But in this version, I implement based on idea discussed at LSF/MM. If my new idea makes sense, I will change it.

Quoted 22 lines · On Wed, 27 Apr 2011 01:25:23 +0900 Minchan Kim <[email protected]> wrote:

This patch defines new APIs to putback the page into previous position of LRU. The idea is simple.

When we try to putback the page into lru list and if friends(prev, next) of the pages still is nearest neighbor, we can insert isolated page into prev's next instead of head of LRU list. So it keeps LRU history without losing the LRU information.

Before :
      LRU POV : H - P1 - P2 - P3 - P4 -T

Isolate P3 :
      LRU POV : H - P1 - P2 - P4 - T

Putback P3 :
      if (P2->next == P4)
              putback(P3, P2);
      So,
      LRU POV : H - P1 - P2 - P3 - P4 -T

For implement, we defines new structure pages_lru which remebers both lru friend pages of isolated one and handling functions.

Quoted 13 lines

But this approach has a problem on contiguous pages.
In this case, my idea can not work since friend pages are isolated, too. It means prev_page->next == next_page always is false and both pages are not LRU any more at that time. It's pointed out by Rik at LSF/MM summit. So for solving the problem, I can change the idea.
I think we don't need both friend(prev, next) pages relation but just consider either prev or next page that it is still same LRU. Worset case in this approach, prev or next page is free and allocate new so it's in head of LRU and our isolated page is located on next of head. But it's almost same situation with current problem. So it doesn't make worse than now and it would be rare. But in this version, I implement based on idea discussed at LSF/MM. If my new idea makes sense, I will change it.

I think using only 'next'(prev?) pointer will be enough.

But this approach has a problem on contiguous pages.
In this case, my idea can not work since friend pages are isolated, too. It means prev_page->next == next_page always is false and both pages are not LRU any more at that time. It's pointed out by Rik at LSF/MM summit. So for solving the problem, I can change the idea.
I think we don't need both friend(prev, next) pages relation but just consider either prev or next page that it is still same LRU.

Any comment?

Quoted 8 lines · Minchan Kim

But this approach has a problem on contiguous pages.
In this case, my idea can not work since friend pages are isolated, too. It means prev_page->next == next_page always is false and both pages are not
LRU any more at that time. It's pointed out by Rik at LSF/MM summit. So for solving the problem, I can change the idea.
I think we don't need both friend(prev, next) pages relation but just consider either prev or next page that it is still same LRU.

Quoted 1 line

Any comment?

If the friend pages are isolated too, then your condition "either prev or next page that it is still same LRU" is likely to be false, no?

This patch defines new APIs to putback the page into previous position of LRU. The idea is simple.

When we try to putback the page into lru list and if friends(prev, next) of the pages still is nearest neighbor, we can insert isolated page into prev's next instead of head of LRU list. So it keeps LRU history without losing the LRU information.

Before :

	LRU POV : H - P1 - P2 - P3 - P4 -T

Isolate P3 :

	LRU POV : H - P1 - P2 - P4 - T

Putback P3 :

	if (P2->next == P4)
		putback(P3, P2);
	So,
	LRU POV : H - P1 - P2 - P3 - P4 -T

For implement, we defines new structure pages_lru which remebers both lru friend pages of isolated one and handling functions.

But this approach has a problem on contiguous pages.
In this case, my idea can not work since friend pages are isolated, too. It means prev_page->next == next_page always is false and both pages are not LRU any more at that time. It's pointed out by Rik at LSF/MM summit. So for solving the problem, I can change the idea.
I think we don't need both friend(prev, next) pages relation but just consider either prev or next page that it is still same LRU. Worset case in this approach, prev or next page is free and allocate new so it's in head of LRU and our isolated page is located on next of head. But it's almost same situation with current problem. So it doesn't make worse than now and it would be rare. But in this version, I implement based on idea discussed at LSF/MM. If my new idea makes sense, I will change it.

Any comment?

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 include/linux/migrate.h  |    2 +
 include/linux/mm_types.h |    7 ++++
 include/linux/swap.h     |    4 ++-
 mm/compaction.c          |    3 +-
 mm/internal.h            |    2 +
 mm/memcontrol.c          |    2 +-
 mm/migrate.c             |   36 +++++++++++++++++++++
 mm/swap.c                |    2 +-
 mm/vmscan.c              |   79 +++++++++++++++++++++++++++++++++++++++++++--
 9 files changed, 129 insertions(+), 8 deletions(-)

include/linux/migrate.h +2 -0

index e39aeec..3aa5ab6 100644
@@ -9,6 +9,7 @@ typedef struct page *new_page_t(struct page *, unsigned long private, int **);
 #ifdef CONFIG_MIGRATION
 #define PAGE_MIGRATION 1
 
+extern void putback_pages_lru(struct list_head *l);
 extern void putback_lru_pages(struct list_head *l);
 extern int migrate_page(struct address_space *,
 			struct page *, struct page *);

@@ -33,6 +34,7 @@ extern int migrate_huge_page_move_mapping(struct address_space *mapping,
 #else
 #define PAGE_MIGRATION 0
 
+static inline void putback_pages_lru(struct list_head *l) {}
 static inline void putback_lru_pages(struct list_head *l) {}
 static inline int migrate_pages(struct list_head *l, new_page_t x,
 		unsigned long private, bool offlining,

include/linux/mm_types.h +0 -0

index ca01ab2..35e80fb 100644
@@ -102,6 +102,13 @@ struct page {
 #endif

include/linux/mm_types.h +0 -0

  * A region containing a mapping of a non-memory backed file under NOMMU
  * conditions.  These are held in a global tree and are pinned by the VMAs that

include/linux/swap.h +3 -1

index baef4ad..4ad0a0c 100644
@@ -227,6 +227,8 @@ extern void rotate_reclaimable_page(struct page *page);
 extern void deactivate_page(struct page *page);
 extern void swap_setup(void);
 
+extern void update_page_reclaim_stat(struct zone *zone, struct page *page,
+                                    int file, int rotated);
 extern void add_page_to_unevictable_list(struct page *page);
 
 /**

@@ -260,7 +262,7 @@ extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
 						struct zone *zone,
 						unsigned long *nr_scanned);
 extern int __isolate_lru_page(struct page *page, int mode, int file,
-				int not_dirty, int not_mapped);
+		int not_dirty, int not_mapped, struct pages_lru *pages_lru);
 extern unsigned long shrink_all_memory(unsigned long nr_pages);
 extern int vm_swappiness;
 extern int remove_mapping(struct address_space *mapping, struct page *page);

mm/compaction.c +2 -1

index 653b02b..c453000 100644
@@ -335,7 +335,8 @@ static unsigned long isolate_migratepages(struct zone *zone,
 		}
 
 		/* Try isolate the page */
-		if (__isolate_lru_page(page, ISOLATE_BOTH, 0, !cc->sync, 0) != 0)
+		if (__isolate_lru_page(page, ISOLATE_BOTH, 0,
+					!cc->sync, 0, NULL) != 0)
 			continue;
 
 		VM_BUG_ON(PageTransCompound(page));

mm/internal.h +2 -0

index d071d38..3c8182c 100644
@@ -43,6 +43,8 @@ extern unsigned long highest_memmap_pfn;
  * in mm/vmscan.c:
  */
 extern int isolate_lru_page(struct page *page);
+extern bool keep_lru_order(struct pages_lru *pages_lru);
+extern void putback_page_to_lru(struct page *page, struct list_head *head);
 extern void putback_lru_page(struct page *page);
 
 /*

mm/memcontrol.c +1 -1

index 471e7fd..92a9046 100644
@@ -1193,7 +1193,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
 			continue;
 
 		scan++;
-		ret = __isolate_lru_page(page, mode, file, 0, 0);
+		ret = __isolate_lru_page(page, mode, file, 0, 0, NULL);
 		switch (ret) {
 		case 0:
 			list_move(&page->lru, dst);

mm/migrate.c +23 -0

index 819d233..9cfb63b 100644
@@ -85,6 +85,42 @@ void putback_lru_pages(struct list_head *l)
 }
 
 /*
+ * This function is almost same iwth putback_lru_pages.
+ * The difference is that function receives struct pages_lru list
+ * and if possible, we add pages into original position of LRU
+ * instead of LRU's head.
+ */
+void putback_pages_lru(struct list_head *l)
+{
+        struct pages_lru *isolated_page;
+        struct pages_lru *isolated_page2;
+        struct page *page;
+
+        list_for_each_entry_safe(isolated_page, isolated_page2, l, lru) {
+                struct zone *zone;
+                page = isolated_page->page;
+                list_del(&isolated_page->lru);
+
+                dec_zone_page_state(page, NR_ISOLATED_ANON +
+                                page_is_file_cache(page));
+
+                zone = page_zone(page);
+                spin_lock_irq(&zone->lru_lock);
+                if (keep_lru_order(isolated_page)) {
+                        putback_page_to_lru(page, &isolated_page->prev_page->lru);

mm/migrate.c +3 -0

+
+
+/*
  * Restore a potential migration pte to a working pte entry
  */
 static int remove_migration_pte(struct page *new, struct vm_area_struct *vma,

mm/swap.c +1 -1

index a83ec5a..0cb15b7 100644
@@ -252,7 +252,7 @@ void rotate_reclaimable_page(struct page *page)
 	}
 }
 
-static void update_page_reclaim_stat(struct zone *zone, struct page *page,
+void update_page_reclaim_stat(struct zone *zone, struct page *page,
 				     int file, int rotated)
 {
 	struct zone_reclaim_stat *reclaim_stat = &zone->reclaim_stat;

mm/vmscan.c +13 -0

index 5196f0c..06a7c9b 100644
@@ -550,6 +550,58 @@ int remove_mapping(struct address_space *mapping, struct page *page)
 	return 0;
 }
 
+/* zone->lru_lock must be hold */
+bool keep_lru_order(struct pages_lru *pages_lru)
+{
+        bool ret = false;
+        struct page *prev, *next;
+
+        if (!pages_lru->prev_page)
+                return ret;
+
+        prev = pages_lru->prev_page;
+        next = pages_lru->next_page;
+
+        if (!PageLRU(prev) || !PageLRU(next))

mm/vmscan.c +52 -4

+
+/**
+ * putback_page_to_lru - put isolated @page onto @head
+ * @page: page to be put back to appropriate lru list
+ * @head: lru position to be put back
+ *
+ * Insert previously isolated @page to appropriate position of lru list
+ * zone->lru_lock must be hold.
+ */
+void putback_page_to_lru(struct page *page, struct list_head *head)
+{
+        int lru, active, file;
+        struct zone *zone = page_zone(page);
+        struct page *prev_page = container_of(head, struct page, lru);
+
+        lru = page_lru(prev_page);
+        active = is_active_lru(lru);
+        file = is_file_lru(lru);
+
+        if (active)
+                SetPageActive(page);
+	else
+		ClearPageActive(page);
+
+        update_page_reclaim_stat(zone, page, file, active);
+        SetPageLRU(page);
+        __add_page_to_lru_list(zone, page, lru, head);
+}
+
 /**
  * putback_lru_page - put previously isolated page onto appropriate LRU list's head
  * @page: page to be put back to appropriate lru list

@@ -959,8 +1011,8 @@ keep_lumpy:
  * not_mapped:	page should be not mapped
  * returns 0 on success, -ve errno on failure.
  */
-int __isolate_lru_page(struct page *page, int mode, int file,
-				int not_dirty, int not_mapped)
+int __isolate_lru_page(struct page *page, int mode, int file, int not_dirty,
+				int not_mapped, struct pages_lru *pages_lru)
 {
 	int ret = -EINVAL;
 

@@ -996,12 +1048,31 @@ int __isolate_lru_page(struct page *page, int mode, int file,
 	ret = -EBUSY;
 
 	if (likely(get_page_unless_zero(page))) {
+		struct zone *zone = page_zone(page);
+		enum lru_list l = page_lru(page);
 		/*
 		 * Be careful not to clear PageLRU until after we're
 		 * sure the page is not being freed elsewhere -- the
 		 * page release code relies on it.
 		 */
 		ClearPageLRU(page);
+
+		if (!pages_lru)
+			goto skip;
+
+		pages_lru->page = page;
+		if (&zone->lru[l].list == pages_lru->lru.prev ||
+			&zone->lru[l].list == pages_lru->lru.next) {
+			pages_lru->prev_page = NULL;
+			pages_lru->next_page = NULL;
+			goto skip;
+		}
+
+		pages_lru->prev_page =
+			list_entry(page->lru.prev, struct page, lru);
+		pages_lru->next_page =
+			list_entry(page->lru.next, struct page, lru);
+skip:
 		ret = 0;
 	}
 

@@ -1054,7 +1125,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 		VM_BUG_ON(!PageLRU(page));
 
 		switch (__isolate_lru_page(page, mode, file,
-					not_dirty, not_mapped)) {
+					not_dirty, not_mapped, NULL)) {
 		case 0:
 			list_move(&page->lru, dst);
 			mem_cgroup_del_lru(page);

@@ -1114,7 +1185,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 				break;
 
 			if (__isolate_lru_page(cursor_page, mode, file,
-					not_dirty, not_mapped) == 0) {
+					not_dirty, not_mapped, NULL) == 0) {
 				list_move(&cursor_page->lru, dst);
 				mem_cgroup_del_lru(cursor_page);
 				nr_taken += hpage_nr_pages(page);

Signature · 1 line

1.7.1

Quoted 82 lines · Minchan Kim

This patch defines new APIs to putback the page into previous position of LRU. The idea is simple.

When we try to putback the page into lru list and if friends(prev, next) of the pages still is nearest neighbor, we can insert isolated page into prev's next instead of head of LRU list. So it keeps LRU history without losing the LRU information.

Before :
      LRU POV : H - P1 - P2 - P3 - P4 -T

Isolate P3 :
      LRU POV : H - P1 - P2 - P4 - T

Putback P3 :
      if (P2->next == P4)
              putback(P3, P2);
      So,
      LRU POV : H - P1 - P2 - P3 - P4 -T

For implement, we defines new structure pages_lru which remebers both lru friend pages of isolated one and handling functions.

But this approach has a problem on contiguous pages.
In this case, my idea can not work since friend pages are isolated, too. It means prev_page->next == next_page always is false and both pages are not LRU any more at that time. It's pointed out by Rik at LSF/MM summit. So for solving the problem, I can change the idea.
I think we don't need both friend(prev, next) pages relation but just consider either prev or next page that it is still same LRU. Worset case in this approach, prev or next page is free and allocate new so it's in head of LRU and our isolated page is located on next of head. But it's almost same situation with current problem. So it doesn't make worse than now and it would be rare. But in this version, I implement based on idea discussed at LSF/MM. If my new idea makes sense, I will change it.

Any comment?

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 include/linux/migrate.h  |    2 +
 include/linux/mm_types.h |    7 ++++
 include/linux/swap.h     |    4 ++-
 mm/compaction.c          |    3 +-
 mm/internal.h            |    2 +
 mm/memcontrol.c          |    2 +-
 mm/migrate.c             |   36 +++++++++++++++++++++
 mm/swap.c                |    2 +-
 mm/vmscan.c              |   79 +++++++++++++++++++++++++++++++++++++++++++--

 9 files changed, 129 insertions(+), 8 deletions(-)

include/linux/migrate.h +2 -0

index e39aeec..3aa5ab6 100644
@@ -9,6 +9,7 @@ typedef struct page *new_page_t(struct page *, unsigned long private, int **);
  #ifdef CONFIG_MIGRATION
  #define PAGE_MIGRATION 1
 
+extern void putback_pages_lru(struct list_head *l);
  extern void putback_lru_pages(struct list_head *l);
  extern int migrate_page(struct address_space *,
                       struct page *, struct page *);

@@ -33,6 +34,7 @@ extern int migrate_huge_page_move_mapping(struct address_space *mapping,
  #else
  #define PAGE_MIGRATION 0
 
+static inline void putback_pages_lru(struct list_head *l) {}
  static inline void putback_lru_pages(struct list_head *l) {}
  static inline int migrate_pages(struct list_head *l, new_page_t x,
               unsigned long private, bool offlining,

include/linux/mm_types.h +0 -0

index ca01ab2..35e80fb 100644
@@ -102,6 +102,13 @@ struct page {
  #endif
  };
 

include/linux/mm_types.h struct page { · line 112

Quoted 8 lines

include/linux/mm_types.h +7 -0

+/* This structure is used for keeping LRU ordering of isolated page */
+struct pages_lru {
+        struct page *page;      /* isolated page */
+        struct page *prev_page; /* previous page of isolate page as LRU order */
+        struct page *next_page; /* next page of isolate page as LRU order */
+        struct list_head lru;
+};
  /*

So this thing has to be allocated from somewhere. We can't put it on the stack as we're already in danger there so we must be using kmalloc. In the reclaim paths, this should be avoided obviously. For compaction, we might hurt the compaction success rates if pages are pinned with control structures. It's something to be wary of.

At LSF/MM, I stated a preference for swapping the source and destination pages in the LRU. This unfortunately means that the LRU

now contains a page in the process of being migrated to and the backout paths for migration failure become a lot more complex. Reclaim should be ok as it'll should fail to lock the page and recycle it in the list. This avoids allocations but I freely admit that I'm not in the position to implement such a thing right now :(

Quoted 100 lines

include/linux/mm_types.h +0 -0

   * A region containing a mapping of a non-memory backed file under NOMMU
   * conditions.  These are held in a global tree and are pinned by the VMAs that

include/linux/swap.h +3 -1

index baef4ad..4ad0a0c 100644
@@ -227,6 +227,8 @@ extern void rotate_reclaimable_page(struct page *page);
  extern void deactivate_page(struct page *page);
  extern void swap_setup(void);
 
+extern void update_page_reclaim_stat(struct zone *zone, struct page *page,
+                                    int file, int rotated);
  extern void add_page_to_unevictable_list(struct page *page);
 
  /**

@@ -260,7 +262,7 @@ extern unsigned long mem_cgroup_shrink_node_zone(struct mem_cgroup *mem,
                                               struct zone *zone,
                                               unsigned long *nr_scanned);
  extern int __isolate_lru_page(struct page *page, int mode, int file,
-                             int not_dirty, int not_mapped);
+             int not_dirty, int not_mapped, struct pages_lru *pages_lru);
  extern unsigned long shrink_all_memory(unsigned long nr_pages);
  extern int vm_swappiness;
  extern int remove_mapping(struct address_space *mapping, struct page *page);

mm/compaction.c +2 -1

index 653b02b..c453000 100644
@@ -335,7 +335,8 @@ static unsigned long isolate_migratepages(struct zone *zone,
               }
 
               /* Try isolate the page */
-             if (__isolate_lru_page(page, ISOLATE_BOTH, 0, !cc->sync, 0) != 0)
+             if (__isolate_lru_page(page, ISOLATE_BOTH, 0,
+                                     !cc->sync, 0, NULL) != 0)
                       continue;
 
               VM_BUG_ON(PageTransCompound(page));

mm/internal.h +2 -0

index d071d38..3c8182c 100644
@@ -43,6 +43,8 @@ extern unsigned long highest_memmap_pfn;
   * in mm/vmscan.c:
   */
  extern int isolate_lru_page(struct page *page);
+extern bool keep_lru_order(struct pages_lru *pages_lru);
+extern void putback_page_to_lru(struct page *page, struct list_head *head);
  extern void putback_lru_page(struct page *page);
 
  /*

mm/memcontrol.c +1 -1

index 471e7fd..92a9046 100644
@@ -1193,7 +1193,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
                       continue;
 
               scan++;
-             ret = __isolate_lru_page(page, mode, file, 0, 0);
+             ret = __isolate_lru_page(page, mode, file, 0, 0, NULL);
               switch (ret) {

              case 0:
                      list_move(&page->lru, dst);

mm/migrate.c +28 -0

index 819d233..9cfb63b 100644
@@ -85,6 +85,42 @@ void putback_lru_pages(struct list_head *l)
  }
 
  /*
+ * This function is almost same iwth putback_lru_pages.
+ * The difference is that function receives struct pages_lru list
+ * and if possible, we add pages into original position of LRU
+ * instead of LRU's head.
+ */
+void putback_pages_lru(struct list_head *l)
+{
+        struct pages_lru *isolated_page;
+        struct pages_lru *isolated_page2;
+        struct page *page;
+
+        list_for_each_entry_safe(isolated_page, isolated_page2, l, lru) {
+                struct zone *zone;
+                page = isolated_page->page;
+                list_del(&isolated_page->lru);
+
+                dec_zone_page_state(page, NR_ISOLATED_ANON +
+                                page_is_file_cache(page));
+
+                zone = page_zone(page);
+                spin_lock_irq(&zone->lru_lock);
+                if (keep_lru_order(isolated_page)) {
+                        putback_page_to_lru(page, &isolated_page->prev_page->lru);
+                        spin_unlock_irq(&zone->lru_lock);
+                }
+                else {
+                        spin_unlock_irq(&zone->lru_lock);
+                        putback_lru_page(page);

mm/migrate.c void putback_lru_pages(struct list_head *l) · line 120

Quoted 5 lines

mm/migrate.c +5 -0

+                }
+
+                kfree(isolated_page);
+        }
+}

I think we also need counters at least at discussion stage to see how successful this is.

For example, early in the system there is a casual relationship between the age of a page and its location in physical memory. The buddy allocator gives pages back in PFN order where possible and there is a loose relationship between when pages get allocated and when they get reclaimed. As compaction is a linear scanner, there is a likelihood (that is highly variable) that physically contiguous pages will have similar positions in the LRU. They'll be isolated at the same time meaning they also won't be put back in order.

This might cease to matter when the system is running for some time but it's a concern.

include/linux/mm_types.h struct page { · line 112

Quoted 8 lines

include/linux/mm_types.h +7 -0

+/* This structure is used for keeping LRU ordering of isolated page */
+struct pages_lru {
+        struct page *page;      /* isolated page */
+        struct page *prev_page; /* previous page of isolate page as LRU order */
+        struct page *next_page; /* next page of isolate page as LRU order */
+        struct list_head lru;
+};
 /*

So this thing has to be allocated from somewhere. We can't put it on the stack as we're already in danger there so we must be using kmalloc. In the reclaim paths, this should be avoided obviously. For compaction, we might hurt the compaction success rates if pages are pinned with control structures. It's something to be wary of.

At LSF/MM, I stated a preference for swapping the source and destination pages in the LRU. This unfortunately means that the LRU now contains a page in the process of being migrated to and the backout paths for migration failure become a lot more complex. Reclaim should be ok as it'll should fail to lock the page and recycle it in the list. This avoids allocations but I freely admit that I'm not in the position to implement such a thing right now :(

Quoted 15 lines

Quoted 8 lines

include/linux/mm_types.h +7 -0

+/* This structure is used for keeping LRU ordering of isolated page */
+struct pages_lru {
+        struct page *page;      /* isolated page */
+        struct page *prev_page; /* previous page of isolate page as LRU order */
+        struct page *next_page; /* next page of isolate page as LRU order */
+        struct list_head lru;
+};

 /*

So this thing has to be allocated from somewhere. We can't put it on the stack as we're already in danger there so we must be using kmalloc. In the reclaim paths, this should be avoided obviously. For compaction, we might hurt the compaction success rates if pages are pinned with control structures. It's something to be wary of.

Quoted 7 lines

At LSF/MM, I stated a preference for swapping the source and destination pages in the LRU. This unfortunately means that the LRU now contains a page in the process of being migrated to and the backout paths for migration failure become a lot more complex. Reclaim should be ok as it'll should fail to lock the page and recycle it in the list. This avoids allocations but I freely admit that I'm not in the position to implement such a thing right now :(

I like swaping to fake page. one way pointer might become dangerous. vmscan can detect fake page and ignore it.

ie,
is_fake_page(page)
{
       if (is_stack_addr((void*)page))
               return true;
       return false;
}

Also, I like to use stack rather than kmalloc in compaction.

Quoted 23 lines · KOSAKI Motohiro

Quoted 22 lines

Quoted 8 lines

(quoted diff) +7 -0

+/* This structure is used for keeping LRU ordering of isolated page */
+struct pages_lru {
+ A  A  A  A struct page *page; A  A  A /* isolated page */
+ A  A  A  A struct page *prev_page; /* previous page of isolate page as LRU order */
+ A  A  A  A struct page *next_page; /* next page of isolate page as LRU order */
+ A  A  A  A struct list_head lru;
+};

A /*

So this thing has to be allocated from somewhere. We can't put it on the stack as we're already in danger there so we must be using kmalloc. In the reclaim paths, this should be avoided obviously. For compaction, we might hurt the compaction success rates if pages are pinned with control structures. It's something to be wary of.

At LSF/MM, I stated a preference for swapping the source and destination pages in the LRU. This unfortunately means that the LRU now contains a page in the process of being migrated to and the backout paths for migration failure become a lot more complex. Reclaim should be ok as it'll should fail to lock the page and recycle it in the list. This avoids allocations but I freely admit that I'm not in the position to implement such a thing right now :(

Quoted 2 lines

I like swaping to fake page. one way pointer might become dangerous. vmscan can detect fake page and ignore it.

I guess it means swapping between migrated-from page and migrated-to page. Right?

If so, migrated-from page is already removed from LRU list and migrated-to page isn't LRU as it's page allocated newly so they don't have any LRU information. How can we swap them? We need space keeps LRU information before removing the page from LRU list. :(

Could you explain in detail about swapping if I miss something?

About one way pointer, I think it's no problem. Worst case I imagine is to put the page in head of LRU list. It means it's same issue now. So it doesn't make worse than now.

Quoted 9 lines

ie,
is_fake_page(page)
{

A  A  A  A if (is_stack_addr((void*)page))
A  A  A  A  A  A  A  A return true;
A  A  A  A return false;

}

Quoted 2 lines

Also, I like to use stack rather than kmalloc in compaction.

Compaction is a procedure of reclaim. As you know, we had a problem about using of stack during reclaim path.
I admit kmalloc-thing isn't good.
I will try to solve the issue as TODO.

Thanks for the review, KOSAKI.

Compaction is good solution to get contiguos page but it makes LRU churing which is not good.
This patch makes that compaction code use in-order putback so after compaction completion, migrated pages are keeping LRU ordering.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 mm/compaction.c |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

mm/compaction.c +4 -3

index a2f6e96..480d2ac 100644
@@ -211,11 +211,11 @@ static void isolate_freepages(struct zone *zone,
 /* Update the number of anon and file isolated pages in the zone */
 static void acct_isolated(struct zone *zone, struct compact_control *cc)
 {
-	struct page *page;
+	struct pages_lru *pages_lru;
 	unsigned int count[NR_LRU_LISTS] = { 0, };
 
-	list_for_each_entry(page, &cc->migratepages, lru) {
-		int lru = page_lru_base_type(page);
+	list_for_each_entry(pages_lru, &cc->migratepages, lru) {
+		int lru = page_lru_base_type(pages_lru->page);
 		count[lru]++;
 	}
 

@@ -281,6 +281,7 @@ static unsigned long isolate_migratepages(struct zone *zone,
 	spin_lock_irq(&zone->lru_lock);
 	for (; low_pfn < end_pfn; low_pfn++) {
 		struct page *page;
+		struct pages_lru *pages_lru;
 		bool locked = true;
 
 		/* give a chance to irqs before checking need_resched() */

Quoted 45 lines · On Wed, 27 Apr 2011 01:25:25 +0900 Minchan Kim <[email protected]> wrote:

Compaction is good solution to get contiguos page but it makes LRU churing which is not good.
This patch makes that compaction code use in-order putback so after compaction completion, migrated pages are keeping LRU ordering.

Cc KOSAKI Motohiro <[email protected]>

Cc Mel Gorman <[email protected]>

Cc Rik van Riel <[email protected]>

Cc Andrea Arcangeli <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
 mm/compaction.c |   22 +++++++++++++++-------

 1 files changed, 15 insertions(+), 7 deletions(-)

mm/compaction.c +4 -3

index a2f6e96..480d2ac 100644
@@ -211,11 +211,11 @@ static void isolate_freepages(struct zone *zone,
  /* Update the number of anon and file isolated pages in the zone */
  static void acct_isolated(struct zone *zone, struct compact_control *cc)
  {
-     struct page *page;
+     struct pages_lru *pages_lru;
       unsigned int count[NR_LRU_LISTS] = { 0, };
 
-     list_for_each_entry(page, &cc->migratepages, lru) {
-             int lru = page_lru_base_type(page);
+     list_for_each_entry(pages_lru, &cc->migratepages, lru) {
+             int lru = page_lru_base_type(pages_lru->page);
               count[lru]++;
       }
 

@@ -281,6 +281,7 @@ static unsigned long isolate_migratepages(struct zone *zone,
       spin_lock_irq(&zone->lru_lock);
       for (; low_pfn < end_pfn; low_pfn++) {
               struct page *page;
+             struct pages_lru *pages_lru;
               bool locked = true;
 
               /* give a chance to irqs before checking need_resched() */

@@ -334,10 +335,16 @@ static unsigned long isolate_migratepages(struct zone *zone,
                       continue;
               }
 

mm/compaction.c static unsigned long isolate_migratepages(struct zone *zone, · line 340

Quoted 3 lines

mm/compaction.c +3 -0

+             pages_lru = kmalloc(sizeof(struct pages_lru), GFP_ATOMIC);
+             if (pages_lru)
+                     continue;

Hmm, can't we use fixed size of statically allocated pages_lru, per-node or per-zone ? I think using kmalloc() in memory reclaim path is risky.