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

[Waiman Long <llong@redhat.com>]

On 4/3/25 12:15 AM, Andrew Morton wrote:
> On Wed,  2 Apr 2025 23:12:12 -0400 Waiman Long <longman@redhat.com> wrote:
>
>> The test_memcontrol selftest consistently fails its test_memcg_low
>> sub-test because of the fact that two of its test child cgroups which
>> have a memmory.low of 0 or an effective memory.low of 0 still have low
>> events generated for them since mem_cgroup_below_low() use the ">="
>> operator when comparing to elow.
>>
>> The simple fix of changing the operator to ">", however, changes the
>> way memory reclaim works quite drastically leading to other failures.
>> 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.
>>
>> With this patch applied, the test_memcg_low sub-test finishes
>> successfully without failure in most cases. Though both test_memcg_low
>> and test_memcg_min sub-tests may fail occasionally if the memory.current
>> values fall outside of the expected ranges.
>>
> Well, maybe the selftest needs to be changed?
Yes, probably some minor adjustment to prevent sporadic failures as much 
as possible. Will look at that.
>
> Please describe this patch in terms of "what is wrong with the code at
> present" and "how that is fixed" and "what is the impact upon
> userspace".
Will do.
>
> Is this change backwardly compatible with existing userspace?

I doubt there will be much impact. There are two cases where the 
behavior will be different.

First of all, if the user doesn't explictly set low/min, it will remain 
0. However, the low/min events may have non-zero value if memory reclaim 
is happening around it. That is certainly unexpected by the users. I 
doubt users will have dependency on a non-zero low/min event count 
because that may or may not happen.

The second case is when we set up an empty cgroup with no task in it. 
The low/min value can be set, but the effective low/min value will be 0. 
Again, low/min events may be triggered and I doubt users will be 
expecting that.

Cheers,
Longman