mm: wait for congestion to clear on all zones

Currently we take a short nap (HZ/10) and wait for congestion to clear before taking another pass with lower priority in balance_pgdat(). But we do that only for the highest zone that we encounter is unbalanced and congested.

This patch changes that to wait on all congested zones in a single pass in the hope that it will save us some scanning that way. Also we take a nap as soon as congested zone is encountered and sc.priority < DEF_PRIORITY - 2 (aka kswapd in trouble).

...

The patch is against the mm tree. Make sure that mm-avoid-calling-pgdat_balanced-needlessly.patch is applied first (not yet in the mmotm tree). Tested on half a dozen systems with different workloads for the last few days, working really well!

Quoted 12 lines · On Wed, 09 Jan 2013 22:41:48 +0100 Zlatko Calusic <[email protected]>…

Currently we take a short nap (HZ/10) and wait for congestion to clear before taking another pass with lower priority in balance_pgdat(). But we do that only for the highest zone that we encounter is unbalanced and congested.

This patch changes that to wait on all congested zones in a single pass in the hope that it will save us some scanning that way. Also we take a nap as soon as congested zone is encountered and sc.priority < DEF_PRIORITY - 2 (aka kswapd in trouble).

...

Quoted 4 lines

The patch is against the mm tree. Make sure that mm-avoid-calling-pgdat_balanced-needlessly.patch is applied first (not yet in the mmotm tree). Tested on half a dozen systems with different workloads for the last few days, working really well!

But what are the user-observable effcets of this change? Less kernel CPU consumption, presumably? Did you quantify it?

Quoted 12 lines · On Wed, 09 Jan 2013 22:41:48 +0100 Zlatko Calusic <[email protected]>…

Currently we take a short nap (HZ/10) and wait for congestion to clear before taking another pass with lower priority in balance_pgdat(). But we do that only for the highest zone that we encounter is unbalanced and congested.

This patch changes that to wait on all congested zones in a single pass in the hope that it will save us some scanning that way. Also we take a nap as soon as congested zone is encountered and sc.priority < DEF_PRIORITY - 2 (aka kswapd in trouble).

...

Quoted 4 lines

The patch is against the mm tree. Make sure that mm-avoid-calling-pgdat_balanced-needlessly.patch is applied first (not yet in the mmotm tree). Tested on half a dozen systems with different workloads for the last few days, working really well!

But what are the user-observable effcets of this change? Less kernel CPU consumption, presumably? Did you quantify it?

From: Zlatko Calusic <[email protected]>

Currently we take a short nap (HZ/10) and wait for congestion to clear before taking another pass with lower priority in balance_pgdat(). But we do that only for the highest zone that we encounter is unbalanced and congested.

This patch changes that to wait on all congested zones in a single pass in the hope that it will save us some scanning that way. Also we take a nap as soon as congested zone is encountered and sc.priority < DEF_PRIORITY - 2 (aka kswapd in trouble).

Cc Mel Gorman <[email protected]>

Cc Hugh Dickins <[email protected]>

Cc Minchan Kim <[email protected]>

Signed-off-by Zlatko Calusic <[email protected]>

---
The patch is against the mm tree. Make sure that mm-avoid-calling-pgdat_balanced-needlessly.patch is applied first (not yet in the mmotm tree). Tested on half a dozen systems with different workloads for the last few days, working really well!

 mm/vmscan.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

mm/vmscan.c +12 -23

index 002ade6..1c5d38a 100644
@@ -2565,7 +2565,6 @@ static unsigned long balance_pgdat(pg_data_t *pgdat, int order,
 							int *classzone_idx)
 {
 	bool pgdat_is_balanced = false;
-	struct zone *unbalanced_zone;
 	int i;
 	int end_zone = 0;	/* Inclusive.  0 = ZONE_DMA */
 	unsigned long total_scanned;

@@ -2596,9 +2595,6 @@ loop_again:
 
 	do {
 		unsigned long lru_pages = 0;
-		int has_under_min_watermark_zone = 0;
-
-		unbalanced_zone = NULL;
 
 		/*
 		 * Scan in the highmem->dma direction for the highest

@@ -2739,15 +2735,20 @@ loop_again:
 			}
 
 			if (!zone_balanced(zone, testorder, 0, end_zone)) {
-				unbalanced_zone = zone;
-				/*
-				 * We are still under min water mark.  This
-				 * means that we have a GFP_ATOMIC allocation
-				 * failure risk. Hurry up!
-				 */
+			    if (total_scanned && sc.priority < DEF_PRIORITY - 2) {
+				/* OK, kswapd is getting into trouble. */
 				if (!zone_watermark_ok_safe(zone, order,
 					    min_wmark_pages(zone), end_zone, 0))
-					has_under_min_watermark_zone = 1;
+				    /*
+				     * We are still under min water mark.
+				     * This means that we have a GFP_ATOMIC
+				     * allocation failure risk. Hurry up!
+				     */
+				    count_vm_event(KSWAPD_SKIP_CONGESTION_WAIT);
+				else
+				    /* Take a nap if a zone is congested. */
+				    wait_iff_congested(zone, BLK_RW_ASYNC, HZ/10);
+			    }
 			} else {
 				/*
 				 * If a zone reaches its high watermark,

@@ -2758,7 +2759,6 @@ loop_again:
 				 */
 				zone_clear_flag(zone, ZONE_CONGESTED);
 			}
-
 		}
 
 		/*

@@ -2776,17 +2776,6 @@ loop_again:
 		}
 
 		/*
-		 * OK, kswapd is getting into trouble.  Take a nap, then take
-		 * another pass across the zones.
-		 */
-		if (total_scanned && (sc.priority < DEF_PRIORITY - 2)) {
-			if (has_under_min_watermark_zone)
-				count_vm_event(KSWAPD_SKIP_CONGESTION_WAIT);
-			else if (unbalanced_zone)
-				wait_iff_congested(unbalanced_zone, BLK_RW_ASYNC, HZ/10);
-		}
-
-		/*
 		 * We do this so kswapd doesn't build up large priorities for
 		 * example when it is freeing in parallel with allocators. It
 		 * matches the direct reclaim path behaviour in terms of impact

Signature · 1 line

1.8.1

Quoted 7 lines · Zlatko Calusic

From: Zlatko Calusic <[email protected]>

Currently we take a short nap (HZ/10) and wait for congestion to clear before taking another pass with lower priority in balance_pgdat(). But we do that only for the highest zone that we encounter is unbalanced and congested.

Quoted 4 lines

This patch changes that to wait on all congested zones in a single pass in the hope that it will save us some scanning that way. Also we take a nap as soon as congested zone is encountered and sc.priority < DEF_PRIORITY - 2 (aka kswapd in trouble).

But you still didn't explain what's the problem you meat and what scenario can get benefit from your change.

I have an observation that without it, under some circumstances that are VERY HARD to repeat (many days need to pass and some stars to align to see the effect), the page cache gets hit hard, 2/3 of it evicted in a split second. And it's not even under high load! So, I'm still monitoring it, but so far the memory utilization really seems better with the patch applied (no more mysterious page cache shootdowns).

Quoted 8 lines · Simon Jeons

Quoted 7 lines · Zlatko Calusic

From: Zlatko Calusic <[email protected]>

Currently we take a short nap (HZ/10) and wait for congestion to clear before taking another pass with lower priority in balance_pgdat(). But we do that only for the highest zone that we encounter is unbalanced and congested.

Quoted 8 lines

Quoted 4 lines

This patch changes that to wait on all congested zones in a single pass in the hope that it will save us some scanning that way. Also we take a nap as soon as congested zone is encountered and sc.priority < DEF_PRIORITY - 2 (aka kswapd in trouble).

But you still didn't explain what's the problem you meat and what scenario can get benefit from your change.

I did in my reply to Andrew. Here's the relevant part:

Quoted 6 lines

I have an observation that without it, under some circumstances that are VERY HARD to repeat (many days need to pass and some stars to align to see the effect), the page cache gets hit hard, 2/3 of it evicted in a split second. And it's not even under high load! So, I'm still monitoring it, but so far the memory utilization really seems better with the patch applied (no more mysterious page cache shootdowns).

The scenario that should get benefit is everyday. I observed problems during light but constant reading from disk (< 10MB/s). And sending that data over the network at the same time. Think backup that compresses data on the fly before pushing it over the network (so it's not very fast).

The trouble is that you can't just fix up a quick benchmark and measure the impact, because many days need to pass for the bug to show up in all it's beauty.

Is there anybody out there who'd like to comment on the patch logic? I.e. do you think that waiting on every congested zone is the more correct solution than waiting on only one (only the highest one, and ignoring the fact that there may be other even more congested zones)?

Regards,

Quoted 35 lines · Zlatko Calusic

Quoted 8 lines · Simon Jeons

Quoted 7 lines · Zlatko Calusic

From: Zlatko Calusic <[email protected]>

Currently we take a short nap (HZ/10) and wait for congestion to clear before taking another pass with lower priority in balance_pgdat(). But we do that only for the highest zone that we encounter is unbalanced and congested.

Quoted 8 lines

Quoted 4 lines

This patch changes that to wait on all congested zones in a single pass in the hope that it will save us some scanning that way. Also we take a nap as soon as congested zone is encountered and sc.priority < DEF_PRIORITY - 2 (aka kswapd in trouble).

But you still didn't explain what's the problem you meat and what scenario can get benefit from your change.

I did in my reply to Andrew. Here's the relevant part:

Quoted 6 lines

I have an observation that without it, under some circumstances that are VERY HARD to repeat (many days need to pass and some stars to align to see the effect), the page cache gets hit hard, 2/3 of it evicted in a split second. And it's not even under high load! So, I'm still monitoring it, but so far the memory utilization really seems better with the patch applied (no more mysterious page cache shootdowns).

The scenario that should get benefit is everyday. I observed problems during light but constant reading from disk (< 10MB/s). And sending that data over the network at the same time. Think backup that compresses data on the fly before pushing it over the network (so it's not very fast).

The trouble is that you can't just fix up a quick benchmark and measure the impact, because many days need to pass for the bug to show up in all it's beauty.

Quoted 4 lines

Is there anybody out there who'd like to comment on the patch logic? I.e. do you think that waiting on every congested zone is the more correct solution than waiting on only one (only the highest one, and ignoring the fact that there may be other even more congested zones)?

What's the benefit of waiting on every congested zone than waiting on only one against your scenario?