[patch v2]vmscan: correctly detect GFP_ATOMIC allocation failure

[Shaohua Li <shaohua.li@intel.com>]

On Sat, 2011-10-08 at 11:35 +0800, Shaohua Li wrote:
> On Sat, 2011-10-08 at 11:19 +0800, David Rientjes wrote:
> > On Sat, 8 Oct 2011, Shaohua Li wrote:
> > 
> > > has_under_min_watermark_zone is used to detect if there is GFP_ATOMIC allocation
> > > failure risk. For a high end_zone, if any zone below or equal to it has min
> > > matermark ok, we have no risk. But current logic is any zone has min watermark
> > > not ok, then we have risk. This is wrong to me.
> > > 
> > > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > > ---
> > >  mm/vmscan.c |    7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > Index: linux/mm/vmscan.c
> > > ===================================================================
> > > --- linux.orig/mm/vmscan.c	2011-09-27 15:09:29.000000000 +0800
> > > +++ linux/mm/vmscan.c	2011-09-27 15:14:45.000000000 +0800
> > > @@ -2463,7 +2463,7 @@ loop_again:
> > >  
> > >  	for (priority = DEF_PRIORITY; priority >= 0; priority--) {
> > >  		unsigned long lru_pages = 0;
> > > -		int has_under_min_watermark_zone = 0;
> > > +		int has_under_min_watermark_zone = 1;
> > 
> > bool
> > 
> > >  
> > >  		/* The swap token gets in the way of swapout... */
> > >  		if (!priority)
> > > @@ -2594,9 +2594,10 @@ loop_again:
> > >  				 * means that we have a GFP_ATOMIC allocation
> > >  				 * failure risk. Hurry up!
> > >  				 */
> > > -				if (!zone_watermark_ok_safe(zone, order,
> > > +				if (has_under_min_watermark_zone &&
> > > +					    zone_watermark_ok_safe(zone, order,
> > >  					    min_wmark_pages(zone), end_zone, 0))
> > > -					has_under_min_watermark_zone = 1;
> > > +					has_under_min_watermark_zone = 0;
> > >  			} else {
> > >  				/*
> > >  				 * If a zone reaches its high watermark,
> > 
> > Ignore checking the min watermark for a moment and consider if all zones 
> > are above the high watermark (a situation where kswapd does not need to 
> > do aggressive reclaim), then has_under_min_watermark_zone doesn't get 
> > cleared and never actually stalls on congestion_wait().  Notice this is 
> > congestion_wait() and not wait_iff_congested(), so the clearing of 
> > ZONE_CONGESTED doesn't prevent this.
> if all zones are above the high watermark, we will have i < 0 when
> detecting the highest imbalanced zone, and the whole loop will end
> without run into congestion_wait().
> or I can add a clearing has_under_min_watermark_zone in the else block
> to be safe.
Subject: vmscan: correctly detect GFP_ATOMIC allocation failure -v2

has_under_min_watermark_zone is used to detect if there is GFP_ATOMIC allocation
failure risk. For a high end_zone, if any zone below or equal to it has min
matermark ok, we have no risk. But current logic is any zone has min watermark
not ok, then we have risk. This is wrong to me.

v2: use bool and clear has_under_min_watermark_zone for zone with watermark ok
as suggested by David Rientjes.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
 mm/vmscan.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Index: linux/mm/vmscan.c
===================================================================
--- linux.orig/mm/vmscan.c	2011-10-08 13:31:49.000000000 +0800
+++ linux/mm/vmscan.c	2011-10-08 13:32:27.000000000 +0800
@@ -2463,7 +2463,7 @@ loop_again:
 
 	for (priority = DEF_PRIORITY; priority >= 0; priority--) {
 		unsigned long lru_pages = 0;
-		int has_under_min_watermark_zone = 0;
+		bool has_under_min_watermark_zone = true;
 
 		/* The swap token gets in the way of swapout... */
 		if (!priority)
@@ -2594,9 +2594,10 @@ loop_again:
 				 * means that we have a GFP_ATOMIC allocation
 				 * failure risk. Hurry up!
 				 */
-				if (!zone_watermark_ok_safe(zone, order,
+				if (has_under_min_watermark_zone &&
+					    zone_watermark_ok_safe(zone, order,
 					    min_wmark_pages(zone), end_zone, 0))
-					has_under_min_watermark_zone = 1;
+					has_under_min_watermark_zone = false;
 			} else {
 				/*
 				 * If a zone reaches its high watermark,
@@ -2608,6 +2609,7 @@ loop_again:
 				zone_clear_flag(zone, ZONE_CONGESTED);
 				if (i <= *classzone_idx)
 					balanced += zone->present_pages;
+				has_under_min_watermark_zone = false;
 			}
 
 		}



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>