On 4/4/25 2:26 PM, Michal Koutný wrote:
Hello Waiman.
On Thu, Apr 03, 2025 at 09:24:34PM -0400, Waiman Long longman@redhat.com wrote:
- 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.)
- 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.
For the test_memcontrol case, a cgroup is created but no task has already been moved into it. So the memory usage is 0. I agree that if a task has ever lived in the cgroup, the usage will not be 0. In that case memory reclaim is certainly justified.
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).
Another alternative is to modify the test to allow non-zero event count even if low is not set.
Cheers, Longman
Michal
[1] https://lore.kernel.org/all/20220322182248.29121-1-mkoutny@suse.com/ [2] https://bugzilla.suse.com/show_bug.cgi?id=1196298