linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Chris Down <chris@chrisdown.name>
To: Yafang Shao <laoar.shao@gmail.com>
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: Fri, 5 Jul 2019 16:52:39 +0100	[thread overview]
Message-ID: <20190705155239.GA18699@chrisdown.name> (raw)
In-Reply-To: <CALOAHbAw5mmpYJb4KRahsjO-Jd0nx1CE+m0LOkciuL6eJtavzQ@mail.gmail.com>

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.

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 :-)

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.


  parent reply	other threads:[~2019-07-05 15:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-05  7:05 [PATCH] mm, memcg: support memory.{min, low} protection in cgroup v1 Yafang Shao
2019-07-05  9:09 ` Michal Hocko
2019-07-05  9:41   ` Yafang Shao
2019-07-05 11:10     ` Michal Hocko
2019-07-05 14:33       ` Yafang Shao
2019-07-05 15:10         ` Brian Foster
2019-07-05 23:39           ` Yafang Shao
2019-07-05 23:52           ` Dave Chinner
2019-07-08 12:15             ` Brian Foster
2019-07-05 19:54         ` Michal Hocko
2019-07-05 23:54           ` Yafang Shao
2019-07-05 15:52     ` Chris Down [this message]
2019-07-05 23:47       ` Yafang Shao
2019-07-06 11:26         ` Chris Down

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190705155239.GA18699@chrisdown.name \
    --to=chris@chrisdown.name \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=laoar.shao@gmail.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=shakeelb@google.com \
    --cc=shaoyafang@didiglobal.com \
    --cc=vdavydov.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).