[Devel] [PATCH v2] ub: correct _ub_get_css() retvalue for dying/stopped containers

[Vasily Averin]

On 9/14/21 9:30 AM, Evgenii Shatokhin wrote:
> On 14.09.2021 00:52, Vasily Averin wrote:
>> Currently __ub_get_css(ub) for dying or stopped container returns
>> ub0-related css as failback. As result read from /proc/bc/<>/ files
>> can show ve0-related info.
>>
>> With this patch, __ub_get_css() will return NULL in such cases,
>> and everyone who called of this function now handles this return value.
>>
>> https://jira.sw.ru/browse/PSBM-123686
>> Signed-off-by: Vasily Averin <vvs at virtuozzo.com>
>> ---
>> v2: removed an extra cehck in end of __ub_get_css(), thanks to eshatokhin@
>>
>>   include/bc/beancounter.h | 10 +++++++-
>>   kernel/bc/beancounter.c  | 65 +++++++++++++++++++++++++++++++++++-------------
>>   kernel/bc/io_prio.c      | 13 ++++++++--
>>   kernel/bc/proc.c         |  3 +++
>>   kernel/bc/vm_pages.c     | 18 +++++++++++---
>>   5 files changed, 85 insertions(+), 24 deletions(-)
>>
>> diff --git a/include/bc/beancounter.h b/include/bc/beancounter.h
>> index e4c5017..4b69a34 100644
>> --- a/include/bc/beancounter.h
>> +++ b/include/bc/beancounter.h
>> @@ -163,6 +163,10 @@ static __always_inline struct cgroup_subsys_state *__ub_get_css(struct user_bean
>>       css = ACCESS_ONCE(ub->ub_bound_css[idx]);
>>       if (likely(css && css_tryget(css))) {
>>           rcu_read_unlock();
>> +        if ((ub != &ub0) && (css == ub0.ub_bound_css[idx])) {
>> +            css_put(css);
>> +            css = NULL;
>> +        }
>>           return css;
>>       }
>>   @@ -183,7 +187,11 @@ static __always_inline struct cgroup_subsys_state *__ub_get_css(struct user_bean
>>       if (css)
>>           css_put(css);
>>   -    css_get(root_css);
>> +    if (ub == &ub0)
>> +        css_get(root_css);
>> +    else
>> +        root_css = NULL;
>> +
>>       return root_css;
>>   }
>>   diff --git a/kernel/bc/beancounter.c b/kernel/bc/beancounter.c
>> index 5e04d6c..ae8d21a 100644
>> --- a/kernel/bc/beancounter.c
>> +++ b/kernel/bc/beancounter.c
>> @@ -117,33 +117,53 @@ int ub_attach_task(struct user_beancounter *ub, struct task_struct *tsk)
>>   {
>>       int ret = 0;
>>       struct user_beancounter *old_ub = tsk->task_bc.exec_ub;
>> -    struct cgroup_subsys_state *css;
>> +    struct cgroup_subsys_state *css, *com, *cob;
>>         if (ub == old_ub)
>>           goto out;
>> +
>> +    ret = -ENODEV;
>> +    com = ub_get_mem_css(old_ub);
>> +    if (!com)
>> +        goto out;
>> +
>> +    cob = ub_get_blkio_css(old_ub);
>> +    if (!cob)
>> +        goto fail_om;
>> +
>>       css = ub_get_mem_css(ub);
>> +    if (!css)
>> +        goto fail_ob;
>> +
>>       ret = cgroup_kernel_attach(css->cgroup, tsk);
>>       css_put(css);
>>       if (ret)
>> -        goto out;
>> +        goto fail_ob;
>> +
>> +    ret = -ENODEV;
>>       css = ub_get_blkio_css(ub);
>> +    if (!css)
>> +        goto fail_blkio;
>> +
>>       ret = cgroup_kernel_attach(css->cgroup, tsk);
>>       css_put(css);
>>       if (ret)
>>           goto fail_blkio;
>> +
>>       ret = cgroup_kernel_attach(ub->css.cgroup, tsk);
>>       if (ret)
>>           goto fail_ub;
>> +
>> +fail_ob:
>> +    css_put(cob);
>> +fail_om:
>> +    css_put(com);
>>   out:
>>       return ret;
>>   fail_ub:
>> -    css = ub_get_blkio_css(old_ub);
>> -    cgroup_kernel_attach(css->cgroup, tsk);
>> -    css_put(css);
>> +    cgroup_kernel_attach(cob->cgroup, tsk);
>>   fail_blkio:
>> -    css = ub_get_mem_css(old_ub);
>> -    cgroup_kernel_attach(css->cgroup, tsk);
>> -    css_put(css);
>> +    cgroup_kernel_attach(com->cgroup, tsk);
>>       goto out;
> 
> I missed it yesterday - if 'goto out' is executed here, css_put() will not be called for 'com' and 'cob'. Maybe, replace it with 'goto fail_ob'?

yes, you're right again.