[PATCH] vmscan: check all_unreclaimable in direct reclaim path

[PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Minchan Kim]

M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
Also he was bisected first bad commit is below

  commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
  Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
  Date:   Fri Jun 4 14:15:05 2010 -0700

     vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure

At first impression, this seemed very strange because the above commit only
chenged function return value and hibernate_preallocate_memory() ignore
return value of shrink_all_memory(). But it's related.

Now, page allocation from hibernation code may enter infinite loop if
the system has highmem. The reasons are that vmscan don't care enough 
OOM case when oom_killer_disabled. 

The problem sequence is following as. 

1. hibernation
2. oom_disable
3. alloc_pages
4. do_try_to_free_pages
       if (scanning_global_lru(sc) && !all_unreclaimable)
               return 1;

If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
shrink_zones maybe return true(ie, all_unreclaimable is true). 
so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.

This patch adds all_unreclaimable check to protect in direct reclaim path, too.
It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.

Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: M. Vefa Bicakci <bicave@superonline.com>
Cc: stable@kernel.org
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
 mm/vmscan.c |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index f620ab3..53b23a7 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1893,12 +1893,11 @@ static void shrink_zone(int priority, struct zone *zone,
  * If a zone is deemed to be full of pinned pages then just give it a light
  * scan then give up on it.
  */
-static bool shrink_zones(int priority, struct zonelist *zonelist,
+static void shrink_zones(int priority, struct zonelist *zonelist,
 					struct scan_control *sc)
 {
 	struct zoneref *z;
 	struct zone *zone;
-	bool all_unreclaimable = true;
 
 	for_each_zone_zonelist_nodemask(zone, z, zonelist,
 					gfp_zone(sc->gfp_mask), sc->nodemask) {
@@ -1916,8 +1915,31 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
 		}
 
 		shrink_zone(priority, zone, sc);
-		all_unreclaimable = false;
 	}
+}
+
+static inline bool all_unreclaimable(struct zonelist *zonelist,
+		struct scan_control *sc)
+{
+	struct zoneref *z;
+	struct zone *zone;
+	bool all_unreclaimable = true;
+
+	if (!scanning_global_lru(sc))
+		return false;
+
+	for_each_zone_zonelist_nodemask(zone, z, zonelist,
+			gfp_zone(sc->gfp_mask), sc->nodemask) {
+		if (!populated_zone(zone))
+			continue;
+		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
+			continue;
+		if (zone->pages_scanned < (zone_reclaimable_pages(zone) * 6)) {
+			all_unreclaimable = false;
+			break;
+		}
+	}
+
 	return all_unreclaimable;
 }
 
@@ -1941,7 +1963,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 					struct scan_control *sc)
 {
 	int priority;
-	bool all_unreclaimable;
 	unsigned long total_scanned = 0;
 	struct reclaim_state *reclaim_state = current->reclaim_state;
 	struct zoneref *z;
@@ -1958,7 +1979,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 		sc->nr_scanned = 0;
 		if (!priority)
 			disable_swap_token();
-		all_unreclaimable = shrink_zones(priority, zonelist, sc);
+		shrink_zones(priority, zonelist, sc);
 		/*
 		 * Don't shrink slabs when reclaiming memory from
 		 * over limit cgroups
@@ -2020,7 +2041,7 @@ out:
 		return sc->nr_reclaimed;
 
 	/* top priority shrink_zones still had more to do? don't OOM, then */
-	if (scanning_global_lru(sc) && !all_unreclaimable)
+	if (!all_unreclaimable(zonelist, sc))
 		return 1;
 
 	return 0;
-- 
1.7.0.5

[PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Minchan Kim]

M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
Also he was bisected first bad commit is below

  commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
  Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
  Date:   Fri Jun 4 14:15:05 2010 -0700

     vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure

At first impression, this seemed very strange because the above commit only
chenged function return value and hibernate_preallocate_memory() ignore
return value of shrink_all_memory(). But it's related.

Now, page allocation from hibernation code may enter infinite loop if
the system has highmem. The reasons are that vmscan don't care enough 
OOM case when oom_killer_disabled. 

The problem sequence is following as. 

1. hibernation
2. oom_disable
3. alloc_pages
4. do_try_to_free_pages
       if (scanning_global_lru(sc) && !all_unreclaimable)
               return 1;

If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
shrink_zones maybe return true(ie, all_unreclaimable is true). 
so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.

This patch adds all_unreclaimable check to protect in direct reclaim path, too.
It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.

Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: M. Vefa Bicakci <bicave@superonline.com>
Cc: stable@kernel.org
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
 mm/vmscan.c |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index f620ab3..53b23a7 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1893,12 +1893,11 @@ static void shrink_zone(int priority, struct zone *zone,
  * If a zone is deemed to be full of pinned pages then just give it a light
  * scan then give up on it.
  */
-static bool shrink_zones(int priority, struct zonelist *zonelist,
+static void shrink_zones(int priority, struct zonelist *zonelist,
 					struct scan_control *sc)
 {
 	struct zoneref *z;
 	struct zone *zone;
-	bool all_unreclaimable = true;
 
 	for_each_zone_zonelist_nodemask(zone, z, zonelist,
 					gfp_zone(sc->gfp_mask), sc->nodemask) {
@@ -1916,8 +1915,31 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
 		}
 
 		shrink_zone(priority, zone, sc);
-		all_unreclaimable = false;
 	}
+}
+
+static inline bool all_unreclaimable(struct zonelist *zonelist,
+		struct scan_control *sc)
+{
+	struct zoneref *z;
+	struct zone *zone;
+	bool all_unreclaimable = true;
+
+	if (!scanning_global_lru(sc))
+		return false;
+
+	for_each_zone_zonelist_nodemask(zone, z, zonelist,
+			gfp_zone(sc->gfp_mask), sc->nodemask) {
+		if (!populated_zone(zone))
+			continue;
+		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
+			continue;
+		if (zone->pages_scanned < (zone_reclaimable_pages(zone) * 6)) {
+			all_unreclaimable = false;
+			break;
+		}
+	}
+
 	return all_unreclaimable;
 }
 
@@ -1941,7 +1963,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 					struct scan_control *sc)
 {
 	int priority;
-	bool all_unreclaimable;
 	unsigned long total_scanned = 0;
 	struct reclaim_state *reclaim_state = current->reclaim_state;
 	struct zoneref *z;
@@ -1958,7 +1979,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 		sc->nr_scanned = 0;
 		if (!priority)
 			disable_swap_token();
-		all_unreclaimable = shrink_zones(priority, zonelist, sc);
+		shrink_zones(priority, zonelist, sc);
 		/*
 		 * Don't shrink slabs when reclaiming memory from
 		 * over limit cgroups
@@ -2020,7 +2041,7 @@ out:
 		return sc->nr_reclaimed;
 
 	/* top priority shrink_zones still had more to do? don't OOM, then */
-	if (scanning_global_lru(sc) && !all_unreclaimable)
+	if (!all_unreclaimable(zonelist, sc))
 		return 1;
 
 	return 0;
-- 
1.7.0.5

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Rafael J. Wysocki]

On Sunday, September 05, 2010, Minchan Kim wrote:
> M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
> 32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
> Also he was bisected first bad commit is below
> 
>   commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
>   Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>   Date:   Fri Jun 4 14:15:05 2010 -0700
> 
>      vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure
> 
> At first impression, this seemed very strange because the above commit only
> chenged function return value and hibernate_preallocate_memory() ignore
> return value of shrink_all_memory(). But it's related.
> 
> Now, page allocation from hibernation code may enter infinite loop if
> the system has highmem. The reasons are that vmscan don't care enough 
> OOM case when oom_killer_disabled. 
> 
> The problem sequence is following as. 
> 
> 1. hibernation
> 2. oom_disable
> 3. alloc_pages
> 4. do_try_to_free_pages
>        if (scanning_global_lru(sc) && !all_unreclaimable)
>                return 1;
> 
> If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
> shrink_zones maybe return true(ie, all_unreclaimable is true). 
> so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.
> 
> This patch adds all_unreclaimable check to protect in direct reclaim path, too.
> It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.
> 
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: M. Vefa Bicakci <bicave@superonline.com>
> Cc: stable@kernel.org
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> ---
>  mm/vmscan.c |   33 +++++++++++++++++++++++++++------
>  1 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f620ab3..53b23a7 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1893,12 +1893,11 @@ static void shrink_zone(int priority, struct zone *zone,
>   * If a zone is deemed to be full of pinned pages then just give it a light
>   * scan then give up on it.
>   */
> -static bool shrink_zones(int priority, struct zonelist *zonelist,
> +static void shrink_zones(int priority, struct zonelist *zonelist,
>  					struct scan_control *sc)
>  {
>  	struct zoneref *z;
>  	struct zone *zone;
> -	bool all_unreclaimable = true;
>  
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist,
>  					gfp_zone(sc->gfp_mask), sc->nodemask) {
> @@ -1916,8 +1915,31 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
>  		}
>  
>  		shrink_zone(priority, zone, sc);
> -		all_unreclaimable = false;
>  	}
> +}
> +
> +static inline bool all_unreclaimable(struct zonelist *zonelist,
> +		struct scan_control *sc)
> +{
> +	struct zoneref *z;
> +	struct zone *zone;
> +	bool all_unreclaimable = true;
> +
> +	if (!scanning_global_lru(sc))
> +		return false;
> +
> +	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> +			gfp_zone(sc->gfp_mask), sc->nodemask) {
> +		if (!populated_zone(zone))
> +			continue;
> +		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> +			continue;
> +		if (zone->pages_scanned < (zone_reclaimable_pages(zone) * 6)) {
> +			all_unreclaimable = false;
> +			break;
> +		}
> +	}
> +
>  	return all_unreclaimable;
>  }
>  
> @@ -1941,7 +1963,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  					struct scan_control *sc)
>  {
>  	int priority;
> -	bool all_unreclaimable;
>  	unsigned long total_scanned = 0;
>  	struct reclaim_state *reclaim_state = current->reclaim_state;
>  	struct zoneref *z;
> @@ -1958,7 +1979,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  		sc->nr_scanned = 0;
>  		if (!priority)
>  			disable_swap_token();
> -		all_unreclaimable = shrink_zones(priority, zonelist, sc);
> +		shrink_zones(priority, zonelist, sc);
>  		/*
>  		 * Don't shrink slabs when reclaiming memory from
>  		 * over limit cgroups
> @@ -2020,7 +2041,7 @@ out:
>  		return sc->nr_reclaimed;
>  
>  	/* top priority shrink_zones still had more to do? don't OOM, then */
> -	if (scanning_global_lru(sc) && !all_unreclaimable)
> +	if (!all_unreclaimable(zonelist, sc))
>  		return 1;
>  
>  	return 0;
> 

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Rafael J. Wysocki]

On Sunday, September 05, 2010, Minchan Kim wrote:
> M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
> 32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
> Also he was bisected first bad commit is below
> 
>   commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
>   Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>   Date:   Fri Jun 4 14:15:05 2010 -0700
> 
>      vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure
> 
> At first impression, this seemed very strange because the above commit only
> chenged function return value and hibernate_preallocate_memory() ignore
> return value of shrink_all_memory(). But it's related.
> 
> Now, page allocation from hibernation code may enter infinite loop if
> the system has highmem. The reasons are that vmscan don't care enough 
> OOM case when oom_killer_disabled. 
> 
> The problem sequence is following as. 
> 
> 1. hibernation
> 2. oom_disable
> 3. alloc_pages
> 4. do_try_to_free_pages
>        if (scanning_global_lru(sc) && !all_unreclaimable)
>                return 1;
> 
> If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
> shrink_zones maybe return true(ie, all_unreclaimable is true). 
> so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.
> 
> This patch adds all_unreclaimable check to protect in direct reclaim path, too.
> It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.
> 
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: M. Vefa Bicakci <bicave@superonline.com>
> Cc: stable@kernel.org
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> ---
>  mm/vmscan.c |   33 +++++++++++++++++++++++++++------
>  1 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f620ab3..53b23a7 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1893,12 +1893,11 @@ static void shrink_zone(int priority, struct zone *zone,
>   * If a zone is deemed to be full of pinned pages then just give it a light
>   * scan then give up on it.
>   */
> -static bool shrink_zones(int priority, struct zonelist *zonelist,
> +static void shrink_zones(int priority, struct zonelist *zonelist,
>  					struct scan_control *sc)
>  {
>  	struct zoneref *z;
>  	struct zone *zone;
> -	bool all_unreclaimable = true;
>  
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist,
>  					gfp_zone(sc->gfp_mask), sc->nodemask) {
> @@ -1916,8 +1915,31 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
>  		}
>  
>  		shrink_zone(priority, zone, sc);
> -		all_unreclaimable = false;
>  	}
> +}
> +
> +static inline bool all_unreclaimable(struct zonelist *zonelist,
> +		struct scan_control *sc)
> +{
> +	struct zoneref *z;
> +	struct zone *zone;
> +	bool all_unreclaimable = true;
> +
> +	if (!scanning_global_lru(sc))
> +		return false;
> +
> +	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> +			gfp_zone(sc->gfp_mask), sc->nodemask) {
> +		if (!populated_zone(zone))
> +			continue;
> +		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> +			continue;
> +		if (zone->pages_scanned < (zone_reclaimable_pages(zone) * 6)) {
> +			all_unreclaimable = false;
> +			break;
> +		}
> +	}
> +
>  	return all_unreclaimable;
>  }
>  
> @@ -1941,7 +1963,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  					struct scan_control *sc)
>  {
>  	int priority;
> -	bool all_unreclaimable;
>  	unsigned long total_scanned = 0;
>  	struct reclaim_state *reclaim_state = current->reclaim_state;
>  	struct zoneref *z;
> @@ -1958,7 +1979,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  		sc->nr_scanned = 0;
>  		if (!priority)
>  			disable_swap_token();
> -		all_unreclaimable = shrink_zones(priority, zonelist, sc);
> +		shrink_zones(priority, zonelist, sc);
>  		/*
>  		 * Don't shrink slabs when reclaiming memory from
>  		 * over limit cgroups
> @@ -2020,7 +2041,7 @@ out:
>  		return sc->nr_reclaimed;
>  
>  	/* top priority shrink_zones still had more to do? don't OOM, then */
> -	if (scanning_global_lru(sc) && !all_unreclaimable)
> +	if (!all_unreclaimable(zonelist, sc))
>  		return 1;
>  
>  	return 0;
> 

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Johannes Weiner]

On Sun, Sep 05, 2010 at 11:40:37PM +0900, Minchan Kim wrote:
> M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
> 32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
> Also he was bisected first bad commit is below
> 
>   commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
>   Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>   Date:   Fri Jun 4 14:15:05 2010 -0700
> 
>      vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure
> 
> At first impression, this seemed very strange because the above commit only
> chenged function return value and hibernate_preallocate_memory() ignore
> return value of shrink_all_memory(). But it's related.
> 
> Now, page allocation from hibernation code may enter infinite loop if
> the system has highmem. The reasons are that vmscan don't care enough 
> OOM case when oom_killer_disabled. 
> 
> The problem sequence is following as. 
> 
> 1. hibernation
> 2. oom_disable
> 3. alloc_pages
> 4. do_try_to_free_pages
>        if (scanning_global_lru(sc) && !all_unreclaimable)
>                return 1;
> 
> If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
> shrink_zones maybe return true(ie, all_unreclaimable is true). 
> so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.
> 
> This patch adds all_unreclaimable check to protect in direct reclaim path, too.
> It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.
> 
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: M. Vefa Bicakci <bicave@superonline.com>
> Cc: stable@kernel.org
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> ---
>  mm/vmscan.c |   33 +++++++++++++++++++++++++++------
>  1 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f620ab3..53b23a7 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1893,12 +1893,11 @@ static void shrink_zone(int priority, struct zone *zone,
>   * If a zone is deemed to be full of pinned pages then just give it a light
>   * scan then give up on it.
>   */
> -static bool shrink_zones(int priority, struct zonelist *zonelist,
> +static void shrink_zones(int priority, struct zonelist *zonelist,
>  					struct scan_control *sc)
>  {
>  	struct zoneref *z;
>  	struct zone *zone;
> -	bool all_unreclaimable = true;
>  
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist,
>  					gfp_zone(sc->gfp_mask), sc->nodemask) {
> @@ -1916,8 +1915,31 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
>  		}
>  
>  		shrink_zone(priority, zone, sc);
> -		all_unreclaimable = false;
>  	}
> +}
> +
> +static inline bool all_unreclaimable(struct zonelist *zonelist,
> +		struct scan_control *sc)
> +{
> +	struct zoneref *z;
> +	struct zone *zone;
> +	bool all_unreclaimable = true;
> +
> +	if (!scanning_global_lru(sc))
> +		return false;
> +
> +	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> +			gfp_zone(sc->gfp_mask), sc->nodemask) {
> +		if (!populated_zone(zone))
> +			continue;
> +		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> +			continue;
> +		if (zone->pages_scanned < (zone_reclaimable_pages(zone) * 6)) {

Small nitpick: kswapd does the same check against the magic number,
could you move it into a separate function?  zone_reclaimable()?

Otherwise,
Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>

> +			all_unreclaimable = false;
> +			break;
> +		}
> +	}
> +
>  	return all_unreclaimable;
>  }
>  
> @@ -1941,7 +1963,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  					struct scan_control *sc)
>  {
>  	int priority;
> -	bool all_unreclaimable;
>  	unsigned long total_scanned = 0;
>  	struct reclaim_state *reclaim_state = current->reclaim_state;
>  	struct zoneref *z;
> @@ -1958,7 +1979,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  		sc->nr_scanned = 0;
>  		if (!priority)
>  			disable_swap_token();
> -		all_unreclaimable = shrink_zones(priority, zonelist, sc);
> +		shrink_zones(priority, zonelist, sc);
>  		/*
>  		 * Don't shrink slabs when reclaiming memory from
>  		 * over limit cgroups
> @@ -2020,7 +2041,7 @@ out:
>  		return sc->nr_reclaimed;
>  
>  	/* top priority shrink_zones still had more to do? don't OOM, then */
> -	if (scanning_global_lru(sc) && !all_unreclaimable)
> +	if (!all_unreclaimable(zonelist, sc))
>  		return 1;
>  
>  	return 0;
> -- 
> 1.7.0.5
> 
> 

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Johannes Weiner]

On Sun, Sep 05, 2010 at 11:40:37PM +0900, Minchan Kim wrote:
> M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
> 32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
> Also he was bisected first bad commit is below
> 
>   commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
>   Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>   Date:   Fri Jun 4 14:15:05 2010 -0700
> 
>      vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure
> 
> At first impression, this seemed very strange because the above commit only
> chenged function return value and hibernate_preallocate_memory() ignore
> return value of shrink_all_memory(). But it's related.
> 
> Now, page allocation from hibernation code may enter infinite loop if
> the system has highmem. The reasons are that vmscan don't care enough 
> OOM case when oom_killer_disabled. 
> 
> The problem sequence is following as. 
> 
> 1. hibernation
> 2. oom_disable
> 3. alloc_pages
> 4. do_try_to_free_pages
>        if (scanning_global_lru(sc) && !all_unreclaimable)
>                return 1;
> 
> If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
> shrink_zones maybe return true(ie, all_unreclaimable is true). 
> so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.
> 
> This patch adds all_unreclaimable check to protect in direct reclaim path, too.
> It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.
> 
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: M. Vefa Bicakci <bicave@superonline.com>
> Cc: stable@kernel.org
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> ---
>  mm/vmscan.c |   33 +++++++++++++++++++++++++++------
>  1 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f620ab3..53b23a7 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1893,12 +1893,11 @@ static void shrink_zone(int priority, struct zone *zone,
>   * If a zone is deemed to be full of pinned pages then just give it a light
>   * scan then give up on it.
>   */
> -static bool shrink_zones(int priority, struct zonelist *zonelist,
> +static void shrink_zones(int priority, struct zonelist *zonelist,
>  					struct scan_control *sc)
>  {
>  	struct zoneref *z;
>  	struct zone *zone;
> -	bool all_unreclaimable = true;
>  
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist,
>  					gfp_zone(sc->gfp_mask), sc->nodemask) {
> @@ -1916,8 +1915,31 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
>  		}
>  
>  		shrink_zone(priority, zone, sc);
> -		all_unreclaimable = false;
>  	}
> +}
> +
> +static inline bool all_unreclaimable(struct zonelist *zonelist,
> +		struct scan_control *sc)
> +{
> +	struct zoneref *z;
> +	struct zone *zone;
> +	bool all_unreclaimable = true;
> +
> +	if (!scanning_global_lru(sc))
> +		return false;
> +
> +	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> +			gfp_zone(sc->gfp_mask), sc->nodemask) {
> +		if (!populated_zone(zone))
> +			continue;
> +		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> +			continue;
> +		if (zone->pages_scanned < (zone_reclaimable_pages(zone) * 6)) {

Small nitpick: kswapd does the same check against the magic number,
could you move it into a separate function?  zone_reclaimable()?

Otherwise,
Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>

> +			all_unreclaimable = false;
> +			break;
> +		}
> +	}
> +
>  	return all_unreclaimable;
>  }
>  
> @@ -1941,7 +1963,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  					struct scan_control *sc)
>  {
>  	int priority;
> -	bool all_unreclaimable;
>  	unsigned long total_scanned = 0;
>  	struct reclaim_state *reclaim_state = current->reclaim_state;
>  	struct zoneref *z;
> @@ -1958,7 +1979,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
>  		sc->nr_scanned = 0;
>  		if (!priority)
>  			disable_swap_token();
> -		all_unreclaimable = shrink_zones(priority, zonelist, sc);
> +		shrink_zones(priority, zonelist, sc);
>  		/*
>  		 * Don't shrink slabs when reclaiming memory from
>  		 * over limit cgroups
> @@ -2020,7 +2041,7 @@ out:
>  		return sc->nr_reclaimed;
>  
>  	/* top priority shrink_zones still had more to do? don't OOM, then */
> -	if (scanning_global_lru(sc) && !all_unreclaimable)
> +	if (!all_unreclaimable(zonelist, sc))
>  		return 1;
>  
>  	return 0;
> -- 
> 1.7.0.5
> 
> 

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Minchan Kim]

On Wed, Sep 08, 2010 at 07:48:31AM +0200, Johannes Weiner wrote:
> On Sun, Sep 05, 2010 at 11:40:37PM +0900, Minchan Kim wrote:
> > M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
> > 32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
> > Also he was bisected first bad commit is below
> > 
> >   commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
> >   Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> >   Date:   Fri Jun 4 14:15:05 2010 -0700
> > 
> >      vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure
> > 
> > At first impression, this seemed very strange because the above commit only
> > chenged function return value and hibernate_preallocate_memory() ignore
> > return value of shrink_all_memory(). But it's related.
> > 
> > Now, page allocation from hibernation code may enter infinite loop if
> > the system has highmem. The reasons are that vmscan don't care enough 
> > OOM case when oom_killer_disabled. 
> > 
> > The problem sequence is following as. 
> > 
> > 1. hibernation
> > 2. oom_disable
> > 3. alloc_pages
> > 4. do_try_to_free_pages
> >        if (scanning_global_lru(sc) && !all_unreclaimable)
> >                return 1;
> > 
> > If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
> > shrink_zones maybe return true(ie, all_unreclaimable is true). 
> > so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.
> > 
> > This patch adds all_unreclaimable check to protect in direct reclaim path, too.
> > It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.
> > 
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: Rik van Riel <riel@redhat.com>
> > Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> > Cc: M. Vefa Bicakci <bicave@superonline.com>
> > Cc: stable@kernel.org
> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> > ---
> >  mm/vmscan.c |   33 +++++++++++++++++++++++++++------
> >  1 files changed, 27 insertions(+), 6 deletions(-)
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index f620ab3..53b23a7 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -1893,12 +1893,11 @@ static void shrink_zone(int priority, struct zone *zone,
> >   * If a zone is deemed to be full of pinned pages then just give it a light
> >   * scan then give up on it.
> >   */
> > -static bool shrink_zones(int priority, struct zonelist *zonelist,
> > +static void shrink_zones(int priority, struct zonelist *zonelist,
> >  					struct scan_control *sc)
> >  {
> >  	struct zoneref *z;
> >  	struct zone *zone;
> > -	bool all_unreclaimable = true;
> >  
> >  	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> >  					gfp_zone(sc->gfp_mask), sc->nodemask) {
> > @@ -1916,8 +1915,31 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
> >  		}
> >  
> >  		shrink_zone(priority, zone, sc);
> > -		all_unreclaimable = false;
> >  	}
> > +}
> > +
> > +static inline bool all_unreclaimable(struct zonelist *zonelist,
> > +		struct scan_control *sc)
> > +{
> > +	struct zoneref *z;
> > +	struct zone *zone;
> > +	bool all_unreclaimable = true;
> > +
> > +	if (!scanning_global_lru(sc))
> > +		return false;
> > +
> > +	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> > +			gfp_zone(sc->gfp_mask), sc->nodemask) {
> > +		if (!populated_zone(zone))
> > +			continue;
> > +		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> > +			continue;
> > +		if (zone->pages_scanned < (zone_reclaimable_pages(zone) * 6)) {
> 
> Small nitpick: kswapd does the same check against the magic number,
> could you move it into a separate function?  zone_reclaimable()?

Nice cleanup. 
Thanks, Hannes. 

> 
> Otherwise,
> Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>
> 

== CUT HERE ==

Changelog 

v2
 * cleanup zone_reclaimable(suggested by Johannes)
 * rebase on mmotm-08-27
 

>From 7b47ffbc9ce89daa44f16bdf447d63ce94dd5f9a Mon Sep 17 00:00:00 2001
From: Minchan Kim <minchan.kim@gmail.com>
Date: Thu, 9 Sep 2010 00:37:52 +0900
Subject: [PATCH v2] vmscan: check all_unreclaimable in direct reclaim path

M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
Also he was bisected first bad commit is below

  commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
  Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
  Date:   Fri Jun 4 14:15:05 2010 -0700

     vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure

At first impression, this seemed very strange because the above commit only
chenged function return value and hibernate_preallocate_memory() ignore
return value of shrink_all_memory(). But it's related.

Now, page allocation from hibernation code may enter infinite loop if
the system has highmem. The reasons are that vmscan don't care enough
OOM case when oom_killer_disabled.

The problem sequence is following as.

1. hibernation
2. oom_disable
3. alloc_pages
4. do_try_to_free_pages
       if (scanning_global_lru(sc) && !all_unreclaimable)
               return 1;

If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
shrink_zones maybe return true(ie, all_unreclaimable is true).
so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.

This patch adds all_unreclaimable check to protect in direct reclaim path, too.
It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.

Cc: Rik van Riel <riel@redhat.com>
Cc: M. Vefa Bicakci <bicave@superonline.com>
Cc: stable@kernel.org
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
---
 mm/vmscan.c |   41 +++++++++++++++++++++++++++++++++--------
 1 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7870893..9a45758 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1877,12 +1877,11 @@ static void shrink_zone(int priority, struct zone *zone,
  * If a zone is deemed to be full of pinned pages then just give it a light
  * scan then give up on it.
  */
-static bool shrink_zones(int priority, struct zonelist *zonelist,
+static void shrink_zones(int priority, struct zonelist *zonelist,
 					struct scan_control *sc)
 {
 	struct zoneref *z;
 	struct zone *zone;
-	bool all_unreclaimable = true;
 
 	for_each_zone_zonelist_nodemask(zone, z, zonelist,
 					gfp_zone(sc->gfp_mask), sc->nodemask) {
@@ -1900,8 +1899,36 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
 		}
 
 		shrink_zone(priority, zone, sc);
-		all_unreclaimable = false;
 	}
+}
+
+static inline bool zone_reclaimable(struct zone *zone)
+{
+	return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
+}
+
+static inline bool all_unreclaimable(struct zonelist *zonelist,
+		struct scan_control *sc)
+{
+	struct zoneref *z;
+	struct zone *zone;
+	bool all_unreclaimable = true;
+
+	if (!scanning_global_lru(sc))
+		return false;
+
+	for_each_zone_zonelist_nodemask(zone, z, zonelist,
+			gfp_zone(sc->gfp_mask), sc->nodemask) {
+		if (!populated_zone(zone))
+			continue;
+		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
+			continue;
+		if (zone_reclaimable(zone)) {
+			all_unreclaimable = false;
+			break;
+		}
+	}
+
 	return all_unreclaimable;
 }
 
@@ -1925,7 +1952,6 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 					struct scan_control *sc)
 {
 	int priority;
-	bool all_unreclaimable;
 	unsigned long total_scanned = 0;
 	struct reclaim_state *reclaim_state = current->reclaim_state;
 	struct zoneref *z;
@@ -1942,7 +1968,7 @@ static unsigned long do_try_to_free_pages(struct zonelist *zonelist,
 		sc->nr_scanned = 0;
 		if (!priority)
 			disable_swap_token();
-		all_unreclaimable = shrink_zones(priority, zonelist, sc);
+		shrink_zones(priority, zonelist, sc);
 		/*
 		 * Don't shrink slabs when reclaiming memory from
 		 * over limit cgroups
@@ -2004,7 +2030,7 @@ out:
 		return sc->nr_reclaimed;
 
 	/* top priority shrink_zones still had more to do? don't OOM, then */
-	if (scanning_global_lru(sc) && !all_unreclaimable)
+	if (!all_unreclaimable(zonelist, sc))
 		return 1;
 
 	return 0;
@@ -2270,8 +2296,7 @@ loop_again:
 			total_scanned += sc.nr_scanned;
 			if (zone->all_unreclaimable)
 				continue;
-			if (nr_slab == 0 &&
-			    zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
+			if (nr_slab == 0 && !zone_reclaimable(zone))
 				zone->all_unreclaimable = 1;
 			/*
 			 * If we've done a decent amount of scanning and
-- 
1.7.0.5

-- 
Kind regards,
Minchan Kim

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Minchan Kim]

On Wed, Sep 08, 2010 at 07:48:31AM +0200, Johannes Weiner wrote:
> On Sun, Sep 05, 2010 at 11:40:37PM +0900, Minchan Kim wrote:
> > M. Vefa Bicakci reported 2.6.35 kernel hang up when hibernation on his
> > 32bit 3GB mem machine. (https://bugzilla.kernel.org/show_bug.cgi?id=16771)
> > Also he was bisected first bad commit is below
> > 
> >   commit bb21c7ce18eff8e6e7877ca1d06c6db719376e3c
> >   Author: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> >   Date:   Fri Jun 4 14:15:05 2010 -0700
> > 
> >      vmscan: fix do_try_to_free_pages() return value when priority==0 reclaim failure
> > 
> > At first impression, this seemed very strange because the above commit only
> > chenged function return value and hibernate_preallocate_memory() ignore
> > return value of shrink_all_memory(). But it's related.
> > 
> > Now, page allocation from hibernation code may enter infinite loop if
> > the system has highmem. The reasons are that vmscan don't care enough 
> > OOM case when oom_killer_disabled. 
> > 
> > The problem sequence is following as. 
> > 
> > 1. hibernation
> > 2. oom_disable
> > 3. alloc_pages
> > 4. do_try_to_free_pages
> >        if (scanning_global_lru(sc) && !all_unreclaimable)
> >                return 1;
> > 
> > If kswapd is not freezed, it would set zone->all_unreclaimable to 1 and then
> > shrink_zones maybe return true(ie, all_unreclaimable is true). 
> > so at last, alloc_pages could go to _nopage_. If it is, it should have no problem.
> > 
> > This patch adds all_unreclaimable check to protect in direct reclaim path, too.
> > It can care of hibernation OOM case and help bailout all_unreclaimable case slightly.
> > 
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: Rik van Riel <riel@redhat.com>
> > Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> > Cc: M. Vefa Bicakci <bicave@superonline.com>
> > Cc: stable@kernel.org
> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> > Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
> > ---
> >  mm/vmscan.c |   33 +++++++++++++++++++++++++++------
> >  1 files changed, 27 insertions(+), 6 deletions(-)
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index f620ab3..53b23a7 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -1893,12 +1893,11 @@ static void shrink_zone(int priority, struct zone *zone,
> >   * If a zone is deemed to be full of pinned pages then just give it a light
> >   * scan then give up on it.
> >   */
> > -static bool shrink_zones(int priority, struct zonelist *zonelist,
> > +static void shrink_zones(int priority, struct zonelist *zonelist,
> >  					struct scan_control *sc)
> >  {
> >  	struct zoneref *z;
> >  	struct zone *zone;
> > -	bool all_unreclaimable = true;
> >  
> >  	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> >  					gfp_zone(sc->gfp_mask), sc->nodemask) {
> > @@ -1916,8 +1915,31 @@ static bool shrink_zones(int priority, struct zonelist *zonelist,
> >  		}
> >  
> >  		shrink_zone(priority, zone, sc);
> > -		all_unreclaimable = false;
> >  	}
> > +}
> > +
> > +static inline bool all_unreclaimable(struct zonelist *zonelist,
> > +		struct scan_control *sc)
> > +{
> > +	struct zoneref *z;
> > +	struct zone *zone;
> > +	bool all_unreclaimable = true;
> > +
> > +	if (!scanning_global_lru(sc))
> > +		return false;
> > +
> > +	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> > +			gfp_zone(sc->gfp_mask), sc->nodemask) {
> > +		if (!populated_zone(zone))
> > +			continue;
> > +		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> > +			continue;
> > +		if (zone->pages_scanned < (zone_reclaimable_pages(zone) * 6)) {
> 
> Small nitpick: kswapd does the same check against the magic number,
> could you move it into a separate function?  zone_reclaimable()?

Nice cleanup. 
Thanks, Hannes. 

> 
> Otherwise,
> Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>
> 

== CUT HERE ==

Changelog 

v2
 * cleanup zone_reclaimable(suggested by Johannes)
 * rebase on mmotm-08-27
 

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Andrew Morton]

On Thu, 9 Sep 2010 00:45:27 +0900
Minchan Kim <minchan.kim@gmail.com> wrote:

> +static inline bool zone_reclaimable(struct zone *zone)
> +{
> +	return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
> +}
> +
> +static inline bool all_unreclaimable(struct zonelist *zonelist,
> +		struct scan_control *sc)
> +{
> +	struct zoneref *z;
> +	struct zone *zone;
> +	bool all_unreclaimable = true;
> +
> +	if (!scanning_global_lru(sc))
> +		return false;
> +
> +	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> +			gfp_zone(sc->gfp_mask), sc->nodemask) {
> +		if (!populated_zone(zone))
> +			continue;
> +		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> +			continue;
> +		if (zone_reclaimable(zone)) {
> +			all_unreclaimable = false;
> +			break;
> +		}
> +	}
> +
>  	return all_unreclaimable;
>  }

Could we have some comments over these functions please?  Why they
exist, what problem they solve, how they solve them, etc.  Stuff which
will be needed for maintaining this code three years from now.

We may as well remove the `inline's too.  gcc will tkae care of that.

> -		if (nr_slab == 0 &&
> -		   zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
> +		if (nr_slab == 0 && !zone_reclaimable(zone))

Extra marks for working out and documenting how we decided on the value
of "6".  Sigh.  It's hopefully in the git record somewhere.

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Andrew Morton]

On Thu, 9 Sep 2010 00:45:27 +0900
Minchan Kim <minchan.kim@gmail.com> wrote:

> +static inline bool zone_reclaimable(struct zone *zone)
> +{
> +	return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
> +}
> +
> +static inline bool all_unreclaimable(struct zonelist *zonelist,
> +		struct scan_control *sc)
> +{
> +	struct zoneref *z;
> +	struct zone *zone;
> +	bool all_unreclaimable = true;
> +
> +	if (!scanning_global_lru(sc))
> +		return false;
> +
> +	for_each_zone_zonelist_nodemask(zone, z, zonelist,
> +			gfp_zone(sc->gfp_mask), sc->nodemask) {
> +		if (!populated_zone(zone))
> +			continue;
> +		if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
> +			continue;
> +		if (zone_reclaimable(zone)) {
> +			all_unreclaimable = false;
> +			break;
> +		}
> +	}
> +
>  	return all_unreclaimable;
>  }

Could we have some comments over these functions please?  Why they
exist, what problem they solve, how they solve them, etc.  Stuff which
will be needed for maintaining this code three years from now.

We may as well remove the `inline's too.  gcc will tkae care of that.

> -		if (nr_slab == 0 &&
> -		   zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
> +		if (nr_slab == 0 && !zone_reclaimable(zone))

Extra marks for working out and documenting how we decided on the value
of "6".  Sigh.  It's hopefully in the git record somewhere.

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Dave Young]

On Thu, Sep 9, 2010 at 6:19 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 9 Sep 2010 00:45:27 +0900
> Minchan Kim <minchan.kim@gmail.com> wrote:
>
>> +static inline bool zone_reclaimable(struct zone *zone)
>> +{
>> +     return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
>> +}
>> +
>> +static inline bool all_unreclaimable(struct zonelist *zonelist,
>> +             struct scan_control *sc)
>> +{
>> +     struct zoneref *z;
>> +     struct zone *zone;
>> +     bool all_unreclaimable = true;
>> +
>> +     if (!scanning_global_lru(sc))
>> +             return false;
>> +
>> +     for_each_zone_zonelist_nodemask(zone, z, zonelist,
>> +                     gfp_zone(sc->gfp_mask), sc->nodemask) {
>> +             if (!populated_zone(zone))
>> +                     continue;
>> +             if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
>> +                     continue;
>> +             if (zone_reclaimable(zone)) {
>> +                     all_unreclaimable = false;
>> +                     break;
>> +             }
>> +     }
>> +
>>       return all_unreclaimable;
>>  }
>
> Could we have some comments over these functions please?  Why they
> exist, what problem they solve, how they solve them, etc.  Stuff which
> will be needed for maintaining this code three years from now.
>
> We may as well remove the `inline's too.  gcc will tkae care of that.
>
>> -             if (nr_slab == 0 &&
>> -                zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
>> +             if (nr_slab == 0 && !zone_reclaimable(zone))
>
> Extra marks for working out and documenting how we decided on the value
> of "6".  Sigh.  It's hopefully in the git record somewhere.

Here it is (necessary to add additional comment?):

commit 4ff1ffb4870b007b86f21e5f27eeb11498c4c077
Author: Nick Piggin <npiggin@suse.de>
Date:   Mon Sep 25 23:31:28 2006 -0700

    [PATCH] oom: reclaim_mapped on oom

    Potentially it takes several scans of the lru lists before we can even start
    reclaiming pages.

    mapped pages, with young ptes can take 2 passes on the active list + one on
    the inactive list.  But reclaim_mapped may not always kick in
instantly, so it
    could take even more than that.

    Raise the threshold for marking a zone as all_unreclaimable from a
factor of 4
    time the pages in the zone to 6.  Introduce a mechanism to force
    reclaim_mapped if we've reached a factor 3 and still haven't made progress.

    Previously, a customer doing stress testing was able to easily OOM the box
    after using only a small fraction of its swap (~100MB).  After the
patches, it
    would only OOM after having used up all swap (~800MB).

>
> --
> 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/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
>



-- 
Regards
dave

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Dave Young]

On Thu, Sep 9, 2010 at 6:19 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 9 Sep 2010 00:45:27 +0900
> Minchan Kim <minchan.kim@gmail.com> wrote:
>
>> +static inline bool zone_reclaimable(struct zone *zone)
>> +{
>> +     return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
>> +}
>> +
>> +static inline bool all_unreclaimable(struct zonelist *zonelist,
>> +             struct scan_control *sc)
>> +{
>> +     struct zoneref *z;
>> +     struct zone *zone;
>> +     bool all_unreclaimable = true;
>> +
>> +     if (!scanning_global_lru(sc))
>> +             return false;
>> +
>> +     for_each_zone_zonelist_nodemask(zone, z, zonelist,
>> +                     gfp_zone(sc->gfp_mask), sc->nodemask) {
>> +             if (!populated_zone(zone))
>> +                     continue;
>> +             if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
>> +                     continue;
>> +             if (zone_reclaimable(zone)) {
>> +                     all_unreclaimable = false;
>> +                     break;
>> +             }
>> +     }
>> +
>>       return all_unreclaimable;
>>  }
>
> Could we have some comments over these functions please?  Why they
> exist, what problem they solve, how they solve them, etc.  Stuff which
> will be needed for maintaining this code three years from now.
>
> We may as well remove the `inline's too.  gcc will tkae care of that

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Minchan Kim]

On Thu, Sep 9, 2010 at 7:19 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 9 Sep 2010 00:45:27 +0900
> Minchan Kim <minchan.kim@gmail.com> wrote:
>
>> +static inline bool zone_reclaimable(struct zone *zone)
>> +{
>> +     return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
>> +}
>> +
>> +static inline bool all_unreclaimable(struct zonelist *zonelist,
>> +             struct scan_control *sc)
>> +{
>> +     struct zoneref *z;
>> +     struct zone *zone;
>> +     bool all_unreclaimable = true;
>> +
>> +     if (!scanning_global_lru(sc))
>> +             return false;
>> +
>> +     for_each_zone_zonelist_nodemask(zone, z, zonelist,
>> +                     gfp_zone(sc->gfp_mask), sc->nodemask) {
>> +             if (!populated_zone(zone))
>> +                     continue;
>> +             if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
>> +                     continue;
>> +             if (zone_reclaimable(zone)) {
>> +                     all_unreclaimable = false;
>> +                     break;
>> +             }
>> +     }
>> +
>>       return all_unreclaimable;
>>  }
>
> Could we have some comments over these functions please?  Why they
> exist, what problem they solve, how they solve them, etc.  Stuff which
> will be needed for maintaining this code three years from now.
>
> We may as well remove the `inline's too.  gcc will tkae care of that.

Okay. I will resend.

>
>> -             if (nr_slab == 0 &&
>> -                zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
>> +             if (nr_slab == 0 && !zone_reclaimable(zone))
>
> Extra marks for working out and documenting how we decided on the value
> of "6".  Sigh.  It's hopefully in the git record somewhere.
>
Originally it is just following as.

                if (zone->pages_scanned > zone->present_pages * 2)
                        zone->all_unreclaimable = 1;

Nick change it with remained lru * 4 [1] and increased 6 [2].
But the description doesn't have why we determine it by "4".
So I can't handle it in my patch.

I don't like undocumented magic value. :(

[1]
commit 9d0aa0f7a99c88dd20bc188756b892f174d93fc1
Author: nickpiggin <nickpiggin>
Date:   Sun Oct 17 16:20:56 2004 +0000

    [PATCH] kswapd lockup fix

    Fix some bugs in the kswapd logic which can cause kswapd lockups.


[2]
commit 4ff1ffb4870b007b86f21e5f27eeb11498c4c077
Author: Nick Piggin <npiggin@suse.de>
Date:   Mon Sep 25 23:31:28 2006 -0700

    [PATCH] oom: reclaim_mapped on oom

-- 
Kind regards,
Minchan Kim

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Minchan Kim]

On Thu, Sep 9, 2010 at 7:19 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 9 Sep 2010 00:45:27 +0900
> Minchan Kim <minchan.kim@gmail.com> wrote:
>
>> +static inline bool zone_reclaimable(struct zone *zone)
>> +{
>> +     return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
>> +}
>> +
>> +static inline bool all_unreclaimable(struct zonelist *zonelist,
>> +             struct scan_control *sc)
>> +{
>> +     struct zoneref *z;
>> +     struct zone *zone;
>> +     bool all_unreclaimable = true;
>> +
>> +     if (!scanning_global_lru(sc))
>> +             return false;
>> +
>> +     for_each_zone_zonelist_nodemask(zone, z, zonelist,
>> +                     gfp_zone(sc->gfp_mask), sc->nodemask) {
>> +             if (!populated_zone(zone))
>> +                     continue;
>> +             if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
>> +                     continue;
>> +             if (zone_reclaimable(zone)) {
>> +                     all_unreclaimable = false;
>> +                     break;
>> +             }
>> +     }
>> +
>>       return all_unreclaimable;
>>  }
>
> Could we have some comments over these functions please?  Why they
> exist, what problem they solve, how they solve them, etc.  Stuff which
> will be needed for maintaining this code three years from now.
>
> We may as well remove the `inline's too.  gcc will tkae care of that.

Okay. I will resend.

>
>> -             if (nr_slab == 0 &&
>> -                zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
>> +             if (nr_slab == 0 && !zone_reclaimable(zone))
>
> Extra marks for working out and documenting how we decided on the value
> of "6".  Sigh.  It's hopefully in the git record somewhere.
>
Originally it is just following as.

                if (zone->pages_scanned > zone->present_pages * 2)
                        zone->all_unreclaimable = 1;

Nick change it with remained lru * 4 [1] and increased 6 [2].
But the description doesn't have why we determine it by "4".
So I can't handle it in my patch.

I don't like undocumented magic value. :(

[1]
commit 9d0aa0f7a99c88dd20bc188756b892f174d93fc1
Author: nickpiggin <nickpiggin>
Date:   Sun Oct 17 16:20:56 2004 +0000

    [PATCH] kswapd lockup fix

    Fix some bugs in the kswapd logic which can cause kswapd lockups.


[2]
commit 4ff1ffb4870b007b86f21e5f27eeb11498c4c077
Author: Nick Piggin <npiggin@suse.de>
Date:   Mon Sep 25 23:31:28 2006 -0700

    [PATCH] oom: reclaim_mapped on oom

-- 
Kind regards,
Minchan Kim

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Minchan Kim]

Thanks, Dave.

On Fri, Sep 10, 2010 at 5:24 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Thu, Sep 9, 2010 at 6:19 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> On Thu, 9 Sep 2010 00:45:27 +0900
>> Minchan Kim <minchan.kim@gmail.com> wrote:
>>
>>> +static inline bool zone_reclaimable(struct zone *zone)
>>> +{
>>> +     return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
>>> +}
>>> +
>>> +static inline bool all_unreclaimable(struct zonelist *zonelist,
>>> +             struct scan_control *sc)
>>> +{
>>> +     struct zoneref *z;
>>> +     struct zone *zone;
>>> +     bool all_unreclaimable = true;
>>> +
>>> +     if (!scanning_global_lru(sc))
>>> +             return false;
>>> +
>>> +     for_each_zone_zonelist_nodemask(zone, z, zonelist,
>>> +                     gfp_zone(sc->gfp_mask), sc->nodemask) {
>>> +             if (!populated_zone(zone))
>>> +                     continue;
>>> +             if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
>>> +                     continue;
>>> +             if (zone_reclaimable(zone)) {
>>> +                     all_unreclaimable = false;
>>> +                     break;
>>> +             }
>>> +     }
>>> +
>>>       return all_unreclaimable;
>>>  }
>>
>> Could we have some comments over these functions please?  Why they
>> exist, what problem they solve, how they solve them, etc.  Stuff which
>> will be needed for maintaining this code three years from now.
>>
>> We may as well remove the `inline's too.  gcc will tkae care of that.
>>
>>> -             if (nr_slab == 0 &&
>>> -                zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
>>> +             if (nr_slab == 0 && !zone_reclaimable(zone))
>>
>> Extra marks for working out and documenting how we decided on the value
>> of "6".  Sigh.  It's hopefully in the git record somewhere.
>
> Here it is (necessary to add additional comment?):
>
> commit 4ff1ffb4870b007b86f21e5f27eeb11498c4c077
> Author: Nick Piggin <npiggin@suse.de>
> Date:   Mon Sep 25 23:31:28 2006 -0700
>
>    [PATCH] oom: reclaim_mapped on oom
>
>    Potentially it takes several scans of the lru lists before we can even start
>    reclaiming pages.
>
>    mapped pages, with young ptes can take 2 passes on the active list + one on
>    the inactive list.  But reclaim_mapped may not always kick in
> instantly, so it
>    could take even more than that.
>
>    Raise the threshold for marking a zone as all_unreclaimable from a
> factor of 4
>    time the pages in the zone to 6.  Introduce a mechanism to force
>    reclaim_mapped if we've reached a factor 3 and still haven't made progress.
>
>    Previously, a customer doing stress testing was able to easily OOM the box
>    after using only a small fraction of its swap (~100MB).  After the
> patches, it
>    would only OOM after having used up all swap (~800MB).
>
>>
>> --
>> 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/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>
>>
>
>
>
> --
> Regards
> dave
>



-- 
Kind regards,
Minchan Kim

Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path

[Minchan Kim]

Thanks, Dave.

On Fri, Sep 10, 2010 at 5:24 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Thu, Sep 9, 2010 at 6:19 AM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> On Thu, 9 Sep 2010 00:45:27 +0900
>> Minchan Kim <minchan.kim@gmail.com> wrote:
>>
>>> +static inline bool zone_reclaimable(struct zone *zone)
>>> +{
>>> +     return zone->pages_scanned < zone_reclaimable_pages(zone) * 6;
>>> +}
>>> +
>>> +static inline bool all_unreclaimable(struct zonelist *zonelist,
>>> +             struct scan_control *sc)
>>> +{
>>> +     struct zoneref *z;
>>> +     struct zone *zone;
>>> +     bool all_unreclaimable = true;
>>> +
>>> +     if (!scanning_global_lru(sc))
>>> +             return false;
>>> +
>>> +     for_each_zone_zonelist_nodemask(zone, z, zonelist,
>>> +                     gfp_zone(sc->gfp_mask), sc->nodemask) {
>>> +             if (!populated_zone(zone))
>>> +                     continue;
>>> +             if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL))
>>> +                     continue;
>>> +             if (zone_reclaimable(zone)) {
>>> +                     all_unreclaimable = false;
>>> +                     break;
>>> +             }
>>> +     }
>>> +
>>>       return all_unreclaimable;
>>>  }
>>
>> Could we have some comments over these functions please?  Why they
>> exist, what problem they solve, how they solve them, etc.  Stuff which
>> will be needed for maintaining this code three years from now.
>>
>> We may as well remove the `inline's too.  gcc will tkae care of that.
>>
>>> -             if (nr_slab == 0 &&
>>> -                zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6))
>>> +             if (nr_slab == 0 && !zone_reclaimable(zone))
>>
>> Extra marks for working out and documenting how we decided on the value
>> of "6".  Sigh.  It's hopefully in the git record somewhere.
>
> Here it is (necessary to add additional comment?):
>
> commit 4ff1ffb4870b007b86f21e5f27eeb11498c4c077
> Author: Nick Piggin <npiggin@suse.de>
> Date:   Mon Sep 25 23:31:28 2006 -0700
>
>    [PATCH] oom: reclaim_mapped on oom
>
>    Potentially it takes several scans of the lru lists before we can even start
>    reclaiming pages.
>
>    mapped pages, with young ptes can take 2 passes on the active list + one on
>    the inactive list.  But reclaim_mapped may not always kick in
> instantly, so it
>    could take even more than that.
>
>    Raise the threshold for marking a zone as all_unreclaimable from a
> factor of 4
>    time the pages in the zone to 6.  Introduce a mechanism to force
>    reclaim_mapped if we've reached a factor 3 and still haven't made progress.
>
>    Previously, a customer doing stress testing was able to easily OOM the box
>    after using only a small fraction of its swap (~100MB).  After the
> patches, it
>    would only OOM after having used up all swap (~800MB).
>
>>
>> --
>> 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/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>
>>
>
>
>
> --
> Regards
> dave
>



-- 
Kind regards,
Minchan Kim

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>