Patchwork [152/241] mm: vmscan: fix endless loop in kswapd balancing

login
register
mail settings
Submitter Ben Hutchings
Date Dec. 15, 2012, 7:56 p.m.
Message ID <1355601411.18807.26.camel@deadeye.wl.decadent.org.uk>
Download mbox | patch
Permalink /patch/206634/
State New
Headers show

Comments

Ben Hutchings - Dec. 15, 2012, 7:56 p.m.
On Thu, 2012-12-13 at 11:58 -0200, Herton Ronaldo Krzesinski wrote:
> 3.5.7.2 -stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Johannes Weiner <hannes@cmpxchg.org>
> 
> commit 60cefed485a02bd99b6299dad70666fe49245da7 upstream.
[...]

Greg, you missed this in 3.{0,4}.y.  I'm attaching the version I used
for 3.2.y, which seems to be applicable to 3.0.y.  One or other of these
should work for 3.4.y.

Ben.
Johannes Weiner - Dec. 18, 2012, 12:16 a.m.
On Sat, Dec 15, 2012 at 07:56:51PM +0000, Ben Hutchings wrote:
> On Thu, 2012-12-13 at 11:58 -0200, Herton Ronaldo Krzesinski wrote:
> > 3.5.7.2 -stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Johannes Weiner <hannes@cmpxchg.org>
> > 
> > commit 60cefed485a02bd99b6299dad70666fe49245da7 upstream.
> [...]
> 
> Greg, you missed this in 3.{0,4}.y.  I'm attaching the version I used
> for 3.2.y, which seems to be applicable to 3.0.y.  One or other of these
> should work for 3.4.y.

Please don't put this in anything earlier than 3.4 as this was the
first version that exhibited the problem.

I apologize for letting it slip into 3.2; a result of failing to
properly mention the origin of the problem in the changelog, tagging
it stable for 3.4+, and noticing the email proposing it for 3.2.  Time
for vacation, I guess... :/
Ben Hutchings - Dec. 27, 2012, 6:41 p.m.
On Mon, 2012-12-17 at 19:16 -0500, Johannes Weiner wrote:
> On Sat, Dec 15, 2012 at 07:56:51PM +0000, Ben Hutchings wrote:
> > On Thu, 2012-12-13 at 11:58 -0200, Herton Ronaldo Krzesinski wrote:
> > > 3.5.7.2 -stable review patch.  If anyone has any objections, please let me know.
> > > 
> > > ------------------
> > > 
> > > From: Johannes Weiner <hannes@cmpxchg.org>
> > > 
> > > commit 60cefed485a02bd99b6299dad70666fe49245da7 upstream.
> > [...]
> > 
> > Greg, you missed this in 3.{0,4}.y.  I'm attaching the version I used
> > for 3.2.y, which seems to be applicable to 3.0.y.  One or other of these
> > should work for 3.4.y.
> 
> Please don't put this in anything earlier than 3.4 as this was the
> first version that exhibited the problem.
> 
> I apologize for letting it slip into 3.2; a result of failing to
> properly mention the origin of the problem in the changelog, tagging
> it stable for 3.4+, and noticing the email proposing it for 3.2.  Time
> for vacation, I guess... :/

OK, I've queued up a patch to revert this in 3.2.

Ben.

Patch

From 39d18dc4b8b0c000fa681cbae10ac3f8a132814b Mon Sep 17 00:00:00 2001
From: Johannes Weiner <hannes@cmpxchg.org>
Date: Thu, 29 Nov 2012 13:54:23 -0800
Subject: [PATCH] mm: vmscan: fix endless loop in kswapd balancing

commit 60cefed485a02bd99b6299dad70666fe49245da7 upstream.

Kswapd does not in all places have the same criteria for a balanced
zone.  Zones are only being reclaimed when their high watermark is
breached, but compaction checks loop over the zonelist again when the
zone does not meet the low watermark plus two times the size of the
allocation.  This gets kswapd stuck in an endless loop over a small
zone, like the DMA zone, where the high watermark is smaller than the
compaction requirement.

Add a function, zone_balanced(), that checks the watermark, and, for
higher order allocations, if compaction has enough free memory.  Then
use it uniformly to check for balanced zones.

This makes sure that when the compaction watermark is not met, at least
reclaim happens and progress is made - or the zone is declared
unreclaimable at some point and skipped entirely.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Reported-by: George Spelvin <linux@horizon.com>
Reported-by: Johannes Hirte <johannes.hirte@fem.tu-ilmenau.de>
Reported-by: Tomas Racek <tracek@redhat.com>
Tested-by: Johannes Hirte <johannes.hirte@fem.tu-ilmenau.de>
Reviewed-by: Rik van Riel <riel@redhat.com>
Cc: Mel Gorman <mel@csn.ul.ie>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---

 mm/vmscan.c |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c

index 313381c..1e4ee1a 100644

--- a/mm/vmscan.c

+++ b/mm/vmscan.c

@@ -2492,6 +2492,19 @@  unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem_cont,

 }
 #endif
 
+static bool zone_balanced(struct zone *zone, int order,

+			  unsigned long balance_gap, int classzone_idx)

+{

+	if (!zone_watermark_ok_safe(zone, order, high_wmark_pages(zone) +

+				    balance_gap, classzone_idx, 0))

+		return false;

+

+	if (COMPACTION_BUILD && order && !compaction_suitable(zone, order))

+		return false;

+

+	return true;

+}

+

 /*
  * pgdat_balanced is used when checking if a node is balanced for high-order
  * allocations. Only zones that meet watermarks and are in a zone allowed
@@ -2551,8 +2564,7 @@  static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,

 			continue;
 		}
 
-		if (!zone_watermark_ok_safe(zone, order, high_wmark_pages(zone),

-							i, 0))

+		if (!zone_balanced(zone, order, 0, i))

 			all_zones_ok = false;
 		else
 			balanced += zone->present_pages;
@@ -2655,8 +2667,7 @@  loop_again:

 				shrink_active_list(SWAP_CLUSTER_MAX, zone,
 							&sc, priority, 0);
 
-			if (!zone_watermark_ok_safe(zone, order,

-					high_wmark_pages(zone), 0, 0)) {

+			if (!zone_balanced(zone, order, 0, 0)) {

 				end_zone = i;
 				break;
 			} else {
@@ -2717,9 +2728,8 @@  loop_again:

 				(zone->present_pages +
 					KSWAPD_ZONE_BALANCE_GAP_RATIO-1) /
 				KSWAPD_ZONE_BALANCE_GAP_RATIO);
-			if (!zone_watermark_ok_safe(zone, order,

-					high_wmark_pages(zone) + balance_gap,

-					end_zone, 0)) {

+			if (!zone_balanced(zone, order,

+					   balance_gap, end_zone)) {

 				shrink_zone(priority, zone, &sc);
 
 				reclaim_state->reclaimed_slab = 0;
@@ -2746,8 +2756,7 @@  loop_again:

 				continue;
 			}
 
-			if (!zone_watermark_ok_safe(zone, order,

-					high_wmark_pages(zone), end_zone, 0)) {

+			if (!zone_balanced(zone, order, 0, end_zone)) {

 				all_zones_ok = 0;
 				/*
 				 * We are still under min water mark.  This