[rh7] vmscan: don't report reclaim progress if there was no progress.

Submitted by Andrey Ryabinin on Oct. 5, 2020, 1:21 p.m.

Details

Message ID 20201005132152.6414-1-aryabinin@virtuozzo.com
State New
Series "vmscan: don't report reclaim progress if there was no progress."
Headers show

Commit Message

Andrey Ryabinin Oct. 5, 2020, 1:21 p.m.
__alloc_pages_slowpath relies on the direct reclaim and did_some_progress
as an indicator that it makes sense to retry allocation rather than
declaring OOM. shrink_zones checks if all zones reclaimable and if
shrink_zone didn't make any progress it prevents from a premature OOM
killer invocation by reporting the progress.
This might happen if the LRU is full of dirty or writeback pages
and direct reclaim cannot clean those up.

zone_reclaimable allows to rescan the reclaimable lists several times
and restart if a page is freed.  This is really subtle behavior and it
might lead to a livelock when a single freed page keeps allocator
looping but the current task will not be able to allocate that single
page.  OOM killer would be more appropriate than looping without any
progress for unbounded amount of time.

Report no progress even if zones are reclaimable as OOM is more appropiate
in that case.

https://jira.sw.ru/browse/PSBM-104900
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 mm/vmscan.c | 24 ------------------------
 1 file changed, 24 deletions(-)

Patch hide | download patch | download mbox

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 13ae9bd1e92e..85622f235e78 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2952,26 +2952,6 @@  static void snapshot_refaults(struct mem_cgroup *root_memcg, struct zone *zone)
        } while ((memcg = mem_cgroup_iter(root_memcg, memcg, NULL)));
 }
 
-/* All zones in zonelist are unreclaimable? */
-static bool all_unreclaimable(struct zonelist *zonelist,
-		struct scan_control *sc)
-{
-	struct zoneref *z;
-	struct zone *zone;
-
-	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))
-			return false;
-	}
-
-	return true;
-}
-
 static void shrink_tcrutches(struct scan_control *scan_ctrl)
 {
 	int nid;
@@ -3097,10 +3077,6 @@  out:
 		goto retry;
 	}
 
-	/* top priority shrink_zones still had more to do? don't OOM, then */
-	if (global_reclaim(sc) && !all_unreclaimable(zonelist, sc))
-		return 1;
-
 	return 0;
 }
 

Comments

Vasily Averin Oct. 9, 2020, 7:22 a.m.
Andrey,
could you please clarify, is it required for vz8 too?

On 10/5/20 4:21 PM, Andrey Ryabinin wrote:
> __alloc_pages_slowpath relies on the direct reclaim and did_some_progress
> as an indicator that it makes sense to retry allocation rather than
> declaring OOM. shrink_zones checks if all zones reclaimable and if
> shrink_zone didn't make any progress it prevents from a premature OOM
> killer invocation by reporting the progress.
> This might happen if the LRU is full of dirty or writeback pages
> and direct reclaim cannot clean those up.
> 
> zone_reclaimable allows to rescan the reclaimable lists several times
> and restart if a page is freed.  This is really subtle behavior and it
> might lead to a livelock when a single freed page keeps allocator
> looping but the current task will not be able to allocate that single
> page.  OOM killer would be more appropriate than looping without any
> progress for unbounded amount of time.
> 
> Report no progress even if zones are reclaimable as OOM is more appropiate
> in that case.
> 
> https://jira.sw.ru/browse/PSBM-104900
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>  mm/vmscan.c | 24 ------------------------
>  1 file changed, 24 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 13ae9bd1e92e..85622f235e78 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2952,26 +2952,6 @@ static void snapshot_refaults(struct mem_cgroup *root_memcg, struct zone *zone)
>         } while ((memcg = mem_cgroup_iter(root_memcg, memcg, NULL)));
>  }
>  
> -/* All zones in zonelist are unreclaimable? */
> -static bool all_unreclaimable(struct zonelist *zonelist,
> -		struct scan_control *sc)
> -{
> -	struct zoneref *z;
> -	struct zone *zone;
> -
> -	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))
> -			return false;
> -	}
> -
> -	return true;
> -}
> -
>  static void shrink_tcrutches(struct scan_control *scan_ctrl)
>  {
>  	int nid;
> @@ -3097,10 +3077,6 @@ out:
>  		goto retry;
>  	}
>  
> -	/* top priority shrink_zones still had more to do? don't OOM, then */
> -	if (global_reclaim(sc) && !all_unreclaimable(zonelist, sc))
> -		return 1;
> -
>  	return 0;
>  }
>  
>
Andrey Ryabinin Oct. 9, 2020, 8:09 a.m.
On 10/9/20 10:22 AM, Vasily Averin wrote:
> Andrey,
> could you please clarify, is it required for vz8 too?
> 

vz8 don't need this. This part  was removed by commit 0a0337e0d1 ("mm, oom: rework oom detection")