[Devel] [PATCH rh7] memcg: ignore memory.low for cgroups w/o tasks

[Kirill Tkhai]

On 26.02.2016 13:09, Konstantin Khorenko wrote:
> Kirill, please review the patch.
> 
> -- 
> Best regards,
> 
> Konstantin Khorenko,
> Virtuozzo Linux Kernel Team
> 
> On 02/26/2016 12:53 PM, Vladimir Davydov wrote:
>> It's OK to set memory.low for a cgroup to infinity. This might be useful
>> if no tasks are supposed to run inside the cgroup itself, but only in
>> its sub-cgroups (e.g. /machine.slice). In this case protection against
>> memory pressure originating on upper levels will be guarded solely by
>> memory.low configuration in sub-cgroups.
>>
>> However, in the current implementation, in contrast to mainstream,
>> charges can appear in a cgroup even if there's no tasks in it - they can
>> be reparented from a dead sub-cgroup. If the cgroup has memory.low set
>> to inf, such reparented charges will not get reclaimed normally on
>> memory pressure, resulting in performance degradation in other cgroups.
>> To avoid that, let's ignore memory.low for cgroups w/o tasks.
>>
>> https://jira.sw.ru/browse/PSBM-44192
>>
>> Signed-off-by: Vladimir Davydov <vdavydov at virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai at virtuozzo.com>

>> ---
>>   mm/memcontrol.c | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index ddd162ae4b3f..6009ff5d1903 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -1607,6 +1607,24 @@ bool mem_cgroup_low(struct mem_cgroup *root, struct mem_cgroup *memcg)
>>       if (res_counter_read_u64(&memcg->res, RES_USAGE) >= memcg->low)
>>           return false;
>>
>> +    /*
>> +     * XXX: It's OK to set memory.low for a cgroup to infinity. This might
>> +     * be useful if no tasks are supposed to run inside the cgroup itself,
>> +     * but only in its sub-cgroups (e.g. /machine.slice). In this case
>> +     * protection against memory pressure originating on upper levels will
>> +     * be guarded solely by memory.low configuration in sub-cgroups.
>> +     *
>> +     * However, in the current implementation, in contrast to mainstream,
>> +     * charges can appear in a cgroup even if there's no tasks in it - they
>> +     * can be reparented from a dead sub-cgroup. If the cgroup has
>> +     * memory.low set to inf, such reparented charges will not get
>> +     * reclaimed normally on memory pressure, resulting in performance
>> +     * degradation in other cgroups. To avoid that, let's ignore memory.low
>> +     * for cgroups w/o tasks.
>> +     */
>> +    if (cgroup_task_count(memcg->css.cgroup) == 0)
>> +        return false;
>> +
>>       while (memcg != root) {
>>           memcg = parent_mem_cgroup(memcg);
>>           if (!memcg)
>>