Re: [PATCH] mm, memcg: support memory.{min, low} protection in cgroup v1

From: Yafang Shao <laoar.shao@gmail.com>
To: Chris Down <chris@chrisdown.name>
Cc: Michal Hocko <mhocko@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Linux MM <linux-mm@kvack.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	 Vladimir Davydov <vdavydov.dev@gmail.com>,
	Shakeel Butt <shakeelb@google.com>,
	 Yafang Shao <shaoyafang@didiglobal.com>
Subject: Re: [PATCH] mm, memcg: support memory.{min, low} protection in cgroup v1
Date: Sat, 6 Jul 2019 07:47:41 +0800	[thread overview]
Message-ID: <CALOAHbBTwas6+rrYAO+OB9R74Ts94T17wojoyOe2+M0CqEbnLw@mail.gmail.com> (raw)
In-Reply-To: <20190705155239.GA18699@chrisdown.name>

On Fri, Jul 5, 2019 at 11:52 PM Chris Down <chris@chrisdown.name> wrote:
>
> Yafang Shao writes:
> >> Cgroup v1 API is considered frozen with new features added only to v2.
> >
> >The facilities support both cgroup v1 and cgroup v2, and what we need
> >to do is only exposing the interface.
> >If the cgroup v1 API is frozen, it will be a pity.
>
> This might be true in the absolute purest technical sense, but not in a
> practical one. Just exposing the memory protection interface without making it
> comprehend v1's API semantics seems a bad move to me -- for example, how it
> (and things like effective protections) interact without the no internal
> process constraint, and certainly many many more things that nobody has
> realised are not going to work yet.
>

Hmm ?
Would be more specific about the issues without the o internal process
constraint ?
The memcg LRU scan/reclaim works fine on both cgroup v1 and cgroup v2,
so the memcg LRU protection should works fine on both cgroup v1 and
cgroup v2 as well.
IOW, if there're some issues without internal process contraint, then
there must be some issues
in cgroup v1 LRU.

> And to that extent, you're really implicitly asking for a lot of work and
> evaluation to be done on memory protections for an interface which is already
> frozen. I'm quite strongly against that.
>
> >Because the interfaces between cgroup v1 and cgroup v2 are changed too
> >much, which is unacceptable by our customer.
>
> The problem is that you're explicitly requesting to use functionality which
> under the hood relies on that new interface while also requesting to not use
> that new interface at the same time :-)
>

I'm sorry that I can't get your point really.
As I said, the facility is already there.
The memcg LRU is not designed for cgroup v2 only.

> While it may superficially work without it, I'm sceptical that simply adding
> memory.low and memory.min to the v1 hierarchy is going to end up with
> reasonable results under scrutiny, or a coherent system or API.

I have finished some simple stress tests, and the result are fine.
Would you pls give me some suggestion on how to test it if you think
there may be some issues ?

Thanks
Yafang