Re: [PATCH] mm: remove '!root_reclaim' checking in should_abort_scan()

[Michal Hocko]

On Thu 12-02-26 11:21:11, zhaoyang.huang wrote:
> From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> 
> Nowadays, ANDROID system replaces madivse with memory.reclaim to implement
> user space memory management which desires to reclaim a certain amount of
> memcg's memory. However, oversized reclaiming and high latency are observed
> as there is no limitation over nr_reclaimed inside try_to_shrink_lruvec
> when MGLRU enabled. Besides, this could also affect all none root_reclaim
> such as reclaim_high etc.
> Since the commit 'b82b530740b9' ("mm: vmscan: restore incremental cgroup
> iteration") introduces sc->memcg_full_walk to limit the walk range of
> mem_cgroup_iter and keep the fairness among the descendants of one memcg.
> This commit would like to make single memcg's scanning more precised by
> removing the criteria of 'if (!root_reclaim)' inside
> should_abort_scan().

This changelog, similar to its previous version is lacking details on
what exactly is going on. How much over-reclaim are we talking about
here? Is this MGLRU specific? Why doesn't our standard over-reclaim
protection work?

> Suggested-by: T.J.Mercier <tjmercier@xxxxxxxxxx>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> ---
>  mm/vmscan.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 670fe9fae5ba..9d900be478ea 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4832,10 +4832,6 @@ static bool should_abort_scan(struct lruvec *lruvec, struct scan_control *sc)
>  	int i;
>  	enum zone_watermarks mark;
>  
> -	/* don't abort memcg reclaim to ensure fairness */
> -	if (!root_reclaim(sc))
> -		return false;
> -
>  	if (sc->nr_reclaimed >= max(sc->nr_to_reclaim, compact_gap(sc->order)))
>  		return true;
>  
> -- 
> 2.25.1
> 

-- 
Michal Hocko
SUSE Labs