Re: [PATCH v2 1/2] memcg: Don't generate low/min events if either low/min or elow/emin is 0

[Michal Koutný]

Hello Waiman.

On Thu, Apr 03, 2025 at 09:24:34PM -0400, Waiman Long <longman@xxxxxxxxxx> wrote:
> 1) memory.low is set to 0, but low events can still be triggered and
>    so the cgroup may have a non-zero low event count. I doubt users are
>    looking for that as they didn't set memory.low at all.

I agree with this reasoning, been there [1] but fix ain't easy (also
consensus of whether such an event should count or not and whether
reclaim should happen or not). (See also [2] where I had tried other
approaches that _didn't_ work.)

> 2) memory.low is set to a non-zero value but the cgroup has no task in
>    it so that it has an effective low value of 0. 

There maybe page cache remaining in the cgroup even with not present
task inside it.

>    Again it may have a non-zero low event count if memory reclaim
>    happens. This is probably not a result expected by the users and it
>    is really doubtful that users will check an empty cgroup with no
>    task in it and expecting some non-zero event counts.

Well, if memory.current > 0, some reclaim events can be justified and
thus expected (e.g. by me).

> The simple and naive fix of changing the operator to ">", however,
> changes the memory reclaim behavior which can lead to other failures
> as low events are needed to facilitate memory reclaim.  So we can't do
> that without some relatively riskier changes in memory reclaim.
> 
> Another simpler alternative is to avoid reporting below_low failure
> if either memory.low or its effective equivalent is 0 which is done
> by this patch specifically for the two failed use cases above.

Admittedly, I haven't seen any complaints from real world about these
events except for this test (which was ported from selftests to LTP
too).

> With this patch applied, the test_memcg_low sub-test finishes
> successfully without failure in most cases.

I'd say the simplest solution to make the test pass without figuring out
what semantics of low events should be correct is not to check the
memory.events:low at all with memory_recursiveprot (this is what was
done in the cloned LTP test).

Michal

[1] https://lore.kernel.org/all/20220322182248.29121-1-mkoutny@xxxxxxxx/
[2] https://bugzilla.suse.com/show_bug.cgi?id=1196298

Attachment: signature.asc
Description: PGP signature