diff mbox

[2/2] mm: vmscan: correct use of pgdat_balanced in sleeping_prematurely

Message ID 1308305213-4657-3-git-send-email-colin.king@canonical.com
State New
Headers show

Commit Message

Colin Ian King June 17, 2011, 10:06 a.m. UTC
From: Johannes Weiner <hannes@cmpxchg.org>

There are a few reports of people experiencing hangs when copying large
amounts of data with kswapd using a large amount of CPU which appear to be
due to recent reclaim changes.  SLUB using high orders is the trigger but
not the root cause as SLUB has been using high orders for a while.  The
root cause was bugs introduced into reclaim which are addressed by the
following two patches.

Patch 1 corrects logic introduced by commit 1741c877 ("mm: kswapd:
        keep kswapd awake for high-order allocations until a percentage of
        the node is balanced") to allow kswapd to go to sleep when
        balanced for high orders.

Patch 2 notes that it is possible for kswapd to miss every
        cond_resched() and updates shrink_slab() so it'll at least reach
        that scheduling point.

Chris Wood reports that these two patches in isolation are sufficient to
prevent the system hanging.  AFAIK, they should also resolve similar hangs
experienced by James Bottomley.

This patch:

Johannes Weiner poined out that the logic in commit 1741c877 ("mm: kswapd:
keep kswapd awake for high-order allocations until a percentage of the
node is balanced") is backwards.  Instead of allowing kswapd to go to
sleep when balancing for high order allocations, it keeps it kswapd
running uselessly.

Signed-off-by: Mel Gorman <mgorman@suse.de>
Reviewed-by: Rik van Riel <riel@redhat.com>
Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Tested-by: Colin King <colin.king@canonical.com>
Cc: Raghavendra D Prabhu <raghu.prabhu13@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Rik van Riel <riel@redhat.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>
Cc: <stable@kernel.org>		[2.6.38+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 mm/vmscan.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Brad Figg June 17, 2011, 10:16 a.m. UTC | #1
On 06/17/2011 11:06 AM, Colin King wrote:
> From: Johannes Weiner<hannes@cmpxchg.org>
>
> There are a few reports of people experiencing hangs when copying large
> amounts of data with kswapd using a large amount of CPU which appear to be
> due to recent reclaim changes.  SLUB using high orders is the trigger but
> not the root cause as SLUB has been using high orders for a while.  The
> root cause was bugs introduced into reclaim which are addressed by the
> following two patches.
>
> Patch 1 corrects logic introduced by commit 1741c877 ("mm: kswapd:
>          keep kswapd awake for high-order allocations until a percentage of
>          the node is balanced") to allow kswapd to go to sleep when
>          balanced for high orders.
>
> Patch 2 notes that it is possible for kswapd to miss every
>          cond_resched() and updates shrink_slab() so it'll at least reach
>          that scheduling point.
>
> Chris Wood reports that these two patches in isolation are sufficient to
> prevent the system hanging.  AFAIK, they should also resolve similar hangs
> experienced by James Bottomley.
>
> This patch:
>
> Johannes Weiner poined out that the logic in commit 1741c877 ("mm: kswapd:
> keep kswapd awake for high-order allocations until a percentage of the
> node is balanced") is backwards.  Instead of allowing kswapd to go to
> sleep when balancing for high order allocations, it keeps it kswapd
> running uselessly.
>
> Signed-off-by: Mel Gorman<mgorman@suse.de>
> Reviewed-by: Rik van Riel<riel@redhat.com>
> Signed-off-by: Johannes Weiner<hannes@cmpxchg.org>
> Reviewed-by: Wu Fengguang<fengguang.wu@intel.com>
> Cc: James Bottomley<James.Bottomley@HansenPartnership.com>
> Tested-by: Colin King<colin.king@canonical.com>
> Cc: Raghavendra D Prabhu<raghu.prabhu13@gmail.com>
> Cc: Jan Kara<jack@suse.cz>
> Cc: Chris Mason<chris.mason@oracle.com>
> Cc: Christoph Lameter<cl@linux.com>
> Cc: Pekka Enberg<penberg@kernel.org>
> Cc: Rik van Riel<riel@redhat.com>
> Reviewed-by: Minchan Kim<minchan.kim@gmail.com>
> Reviewed-by: Wu Fengguang<fengguang.wu@intel.com>
> Cc:<stable@kernel.org>		[2.6.38+]
> Signed-off-by: Andrew Morton<akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds<torvalds@linux-foundation.org>
> ---
>   mm/vmscan.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 648aab8..a74bf72 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2291,7 +2291,7 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,
>   	 * must be balanced
>   	 */
>   	if (order)
> -		return pgdat_balanced(pgdat, balanced, classzone_idx);
> +		return !pgdat_balanced(pgdat, balanced, classzone_idx);
>   	else
>   		return !all_zones_ok;
>   }

Acked-by: Brad Figg <brad.figg@canonical.com>
Herton Ronaldo Krzesinski June 17, 2011, 12:17 p.m. UTC | #2
On Fri, Jun 17, 2011 at 11:06:53AM +0100, Colin King wrote:
> From: Johannes Weiner <hannes@cmpxchg.org>
> 
> There are a few reports of people experiencing hangs when copying large
> amounts of data with kswapd using a large amount of CPU which appear to be
> due to recent reclaim changes.  SLUB using high orders is the trigger but
> not the root cause as SLUB has been using high orders for a while.  The
> root cause was bugs introduced into reclaim which are addressed by the
> following two patches.
> 
> Patch 1 corrects logic introduced by commit 1741c877 ("mm: kswapd:
>         keep kswapd awake for high-order allocations until a percentage of
>         the node is balanced") to allow kswapd to go to sleep when
>         balanced for high orders.
> 
> Patch 2 notes that it is possible for kswapd to miss every
>         cond_resched() and updates shrink_slab() so it'll at least reach
>         that scheduling point.
> 
> Chris Wood reports that these two patches in isolation are sufficient to
> prevent the system hanging.  AFAIK, they should also resolve similar hangs
> experienced by James Bottomley.
> 
> This patch:
> 
> Johannes Weiner poined out that the logic in commit 1741c877 ("mm: kswapd:
> keep kswapd awake for high-order allocations until a percentage of the
> node is balanced") is backwards.  Instead of allowing kswapd to go to
> sleep when balancing for high order allocations, it keeps it kswapd
> running uselessly.
> 
> Signed-off-by: Mel Gorman <mgorman@suse.de>
> Reviewed-by: Rik van Riel <riel@redhat.com>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Tested-by: Colin King <colin.king@canonical.com>
> Cc: Raghavendra D Prabhu <raghu.prabhu13@gmail.com>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Chris Mason <chris.mason@oracle.com>
> Cc: Christoph Lameter <cl@linux.com>
> Cc: Pekka Enberg <penberg@kernel.org>
> Cc: Rik van Riel <riel@redhat.com>
> Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
> Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>
> Cc: <stable@kernel.org>		[2.6.38+]
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>  mm/vmscan.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 648aab8..a74bf72 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2291,7 +2291,7 @@ static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,
>  	 * must be balanced
>  	 */
>  	if (order)
> -		return pgdat_balanced(pgdat, balanced, classzone_idx);
> +		return !pgdat_balanced(pgdat, balanced, classzone_idx);
>  	else
>  		return !all_zones_ok;
>  }

Acked-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>

> -- 
> 1.7.0.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
diff mbox

Patch

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 648aab8..a74bf72 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2291,7 +2291,7 @@  static bool sleeping_prematurely(pg_data_t *pgdat, int order, long remaining,
 	 * must be balanced
 	 */
 	if (order)
-		return pgdat_balanced(pgdat, balanced, classzone_idx);
+		return !pgdat_balanced(pgdat, balanced, classzone_idx);
 	else
 		return !all_zones_ok;
 }