Re: [PATCH v2 2/2] compaction: fix COMPACTPAGEFAILED counting

Previous thread: [PATCH] x86, vmware: fix compilation failure due lack of preset_lpj variable by Otavio Salvador on Tuesday, August 24, 2010 - 8:19 am. (1 message)

Next thread: [PATCH 00/18] Add new semantic patches and reorganize existing ones by Nicolas Palix on Tuesday, August 24, 2010 - 8:38 am. (20 messages)
From: Minchan Kim
Date: Tuesday, August 24, 2010 - 8:31 am

Iram reported compaction's too_many_isolated loops forever.
(http://www.spinics.net/lists/linux-mm/msg08123.html)

The meminfo of situation happened was inactive anon is zero.
That's because the system has no memory pressure until then.
While all anon pages was in active lru, compaction could select
active lru as well as inactive lru. That's different things
with vmscan's isolated. So we has been two too_many_isolated.

While compaction can isolated pages in both active and inactive,
current implementation of too_many_isolated only considers inactive.
It made Iram's problem.

This patch handles active and inactive with fair.
That's because we can't expect where from and how many compaction would
isolated pages.

This patch changes (nr_isolated > nr_inactive) with
nr_isolated > (nr_active + nr_inactive) / 2.

Cc: Iram Shahzad <iram.shahzad@jp.fujitsu.com>
Acked-by: Mel Gorman <mel@csn.ul.ie>
Acked-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
 mm/compaction.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 94cce51..4d709ee 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -214,15 +214,16 @@ static void acct_isolated(struct zone *zone, struct compact_control *cc)
 /* Similar to reclaim, but different enough that they don't share logic */
 static bool too_many_isolated(struct zone *zone)
 {
-
-	unsigned long inactive, isolated;
+	unsigned long active, inactive, isolated;
 
 	inactive = zone_page_state(zone, NR_INACTIVE_FILE) +
 					zone_page_state(zone, NR_INACTIVE_ANON);
+	active = zone_page_state(zone, NR_ACTIVE_FILE) +
+					zone_page_state(zone, NR_ACTIVE_ANON);
 	isolated = zone_page_state(zone, NR_ISOLATED_FILE) +
 					zone_page_state(zone, NR_ISOLATED_ANON);
 
-	return isolated > inactive;
+	return isolated > (inactive + active) / 2;
 }
 
 /*
-- 
1.7.0.5

--

From: Minchan Kim
Date: Tuesday, August 24, 2010 - 8:31 am

Now update_nr_listpages doesn't have a role. That's because
lists passed is always empty just after calling migrate_pages.
The migrate_pages cleans up page list which have failed to migrate
before returning by aaa994b3.

 [PATCH] page migration: handle freeing of pages in migrate_pages()

 Do not leave pages on the lists passed to migrate_pages().  Seems that we will
 not need any postprocessing of pages.  This will simplify the handling of
 pages by the callers of migrate_pages().

At that time, we thought we don't need any postprocessing of pages.
But the situation is changed. The compaction need to know the number of
failed to migrate for COMPACTPAGEFAILED stat

This patch introduces new argument 'cleanup' to migrate_pages.
This patch make new rule for caller of migrate_pages to call putback_lru_pages.
So caller need to clean up the lists so it has a chance to postprocess the pages.

Cc: Hugh Dickins <hughd@google.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
 mm/memory-failure.c |    1 +
 mm/memory_hotplug.c |    2 ++
 mm/mempolicy.c      |   10 ++++++++--
 mm/migrate.c        |   12 +++++++-----
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 9c26eec..5267861 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1339,6 +1339,7 @@ int soft_offline_page(struct page *page, int flags)
 		list_add(&page->lru, &pagelist);
 		ret = migrate_pages(&pagelist, new_page, MPOL_MF_MOVE_ALL, 0);
 		if (ret) {
+			putback_lru_pages(&pagelist);
 			pr_debug("soft offline: %#lx: migration failed %d, type %lx\n",
 				pfn, ret, page->flags);
 			if (ret > 0)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index a4cfcdc..2638079 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -731,6 +731,8 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
 		goto ...
From: Minchan Kim
Date: Tuesday, August 24, 2010 - 8:42 am

Broken description. Will resend 

-- 
Kind regards,
Minchan Kim
--

From: Mel Gorman
Date: Thursday, August 26, 2010 - 2:03 am

Please send this patch on its own as it looks like it should be merged and
arguably is a stable candidate for 2.6.35. Alternatively, Andrew, can you pick
up just this patch? It seems unrelated to the second patch on COMPACTPAGEFAILED.

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

From: Minchan Kim
Date: Thursday, August 26, 2010 - 2:39 am

I thought it's not urgent and next patch would apply based on this
patch without HUNK.
If Andrew doesn't have a response, I will resend as a standalone.
Thanks.

-- 
Kind regards,
Minchan Kim
--

From: Mel Gorman
Date: Thursday, August 26, 2010 - 2:43 am

Well it fixes a known bug so I would consider it a little urgent. It's not

Thanks.

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

Previous thread: [PATCH] x86, vmware: fix compilation failure due lack of preset_lpj variable by Otavio Salvador on Tuesday, August 24, 2010 - 8:19 am. (1 message)

Next thread: [PATCH 00/18] Add new semantic patches and reorganize existing ones by Nicolas Palix on Tuesday, August 24, 2010 - 8:38 am. (20 messages)