On Mon 21-09-20 19:23:01, Yafang Shao wrote:

On Mon, Sep 21, 2020 at 7:05 PM Michal Hocko [off-list ref] wrote:

quoted

On Mon 21-09-20 18:55:40, Yafang Shao wrote:

quoted

On Mon, Sep 21, 2020 at 4:12 PM Michal Hocko [off-list ref] wrote:

quoted

On Mon 21-09-20 16:02:55, zangchunxin@bytedance.com wrote:

quoted

From: Chunxin Zang <redacted>

In the cgroup v1, we have 'force_mepty' interface. This is very
useful for userspace to actively release memory. But the cgroup
v2 does not.

This patch reuse cgroup v1's function, but have a new name for
the interface. Because I think 'drop_cache' may be is easier to
understand :)

This should really explain a usecase. Global drop_caches is a terrible
interface and it has caused many problems in the past. People have
learned to use it as a remedy to any problem they might see and cause
other problems without realizing that. This is the reason why we even
log each attempt to drop caches.

I would rather not repeat the same mistake on the memcg level unless
there is a very strong reason for it.

I think we'd better add these comments above the function
mem_cgroup_force_empty() to explain why we don't want to expose this
interface in cgroup2, otherwise people will continue to send this
proposal without any strong reason.

I do not mind people sending this proposal.  "V1 used to have an
interface, we need it in v2 as well" is not really viable without
providing more reasoning on the specific usecase.

_Any_ patch should have a proper justification. This is nothing really
new to the process and I am wondering why this is coming as a surprise.

Container users always want to drop cache in a specific container,
because they used to use drop_caches to fix memory pressure issues.

This is exactly the kind of problems we have seen in the past. There
should be zero reason to addre potential reclaim problems by dropping
page cache on the floor. There is a huge cargo cult about this
procedure and I have seen numerous reports when people complained about
performance afterwards just to learn that the dropped page cache was one
of the resons for that.

Although drop_caches can cause some unexpected issues, it could also
fix some issues.

"Some issues" is way too general. We really want to learn about those
issues and address them properly.

So container users want to use it in containers as
well.
If this feature is not implemented in cgroup, they will ask you why
but there is no explanation in the kernel.

There is no usecase that would really require it so far.

Regarding the memory.high, it is not perfect as well, because you have
to set it to 0 to drop_caches, and the processes in the containers
have to reclaim pages as well because they reach the memory.high, but
memory.force_empty won't make other processes to reclaim.

Since 536d3bf261a2 ("mm: memcontrol: avoid workload stalls when lowering
memory.high") the limit is set after the reclaim so the race window when
somebody would be pushed to high limit reclaim is reduced. But I do
agree this is just a workaround.

That doesn't mean I agree to add this interface, while I really mean
that if we discard one feature we'd better explain why.

We need to understand why somebody wants an interface because once it is
added it will have to be maintained for ever.
-- 
Michal Hocko
SUSE Labs