Re: [PATCH 0/3] mm: improve proportional memcg protection

[Johannes Weiner <hannes@cmpxchg.org>]

On Sat, Apr 25, 2020 at 11:24:15AM -0400, Yafang Shao wrote:
> Since proportional memory.{min, low} reclaim is introduced in
> commit 9783aa9917f8 ("mm, memcg: proportional memory.{low,min} reclaim"),
> it have been proved that the proportional reclaim is hard to understand and
> the issues caused by it is harder to understand.[1]. That dilemma faced by
> us is caused by that the proportional reclaim mixed up memcg and the
> reclaim context.
> 
> In proportional reclaim, the whole reclaim context - includes the memcg
> to be reclaimed and the reclaimer, should be considered, rather than
> memcg only.
> 
> To make it clear, a new member 'protection' is introduced in the reclaim
> context (struct shrink_control) to replace mem_cgroup_protection(). This
> one is set when we check whether the memcg is protected or not.

This patch series makes the code harder to understand.

It's already tricky to understand ownership and lifetime/validity of
struct scan_control members when it's just vmscan.c functions touching
them. Expanding the scope to the entire MM is a bad idea.

Splitting up sc->memcg_low_reclaim handling and the associated restart
logic makes control flow harder to follow as well.

The problem with the current code is a suboptimal interface between
the memcg protection calculation and the reclaim control flow. We need
to clean that up, not remove the interface and thereby all clarity on
who handles which information.