linux-kernel - Re: [PATCH v1 -next 1/3] cgroup/freezer: Reduce redundant traversal for cgroup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <98d20bd5-5b07-46c1-b098-0d883255484e@huaweicloud.com>
Date: Tue, 10 Sep 2024 10:39:25 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Michal Koutný <mkoutny@...e.com>,
 Chen Ridong <chenridong@...wei.com>
Cc: tj@...nel.org, lizefan.x@...edance.com, hannes@...xchg.org,
 longman@...hat.com, adityakali@...gle.com, sergeh@...nel.org,
 cgroups@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 -next 1/3] cgroup/freezer: Reduce redundant traversal
 for cgroup_freeze



On 2024/9/10 3:01, Michal Koutný wrote:
> On Thu, Sep 05, 2024 at 01:41:28PM GMT, Chen Ridong <chenridong@...wei.com> wrote:
>> Whether a cgroup is frozen is determined solely by whether it is set to
>> to be frozen and whether its parent is frozen. Currently, when is cgroup
>> is frozen or unfrozen, it iterates through the entire subtree to freeze
>> or unfreeze its descentdants.
> 
> It's more to maintain the numeric freeze "layers".
> 
>> However, this is unesessary for a cgroup that does not change its
>> effective frozen status.
> 
> True.
> 
>> +static inline void cgroup_update_efreeze(struct cgroup *cgrp)
>> +{
>> +	struct cgroup *parent = cgroup_parent(cgrp);
>> +	bool p_e = false;
>> +
>> +	if (parent)
>> +		p_e = parent->freezer.e_freeze;
>> +
>> +	cgrp->freezer.e_freeze = cgrp->freezer.freeze | p_e;
> 
> Better be || on bools
> 

Thanks you, that is right.

> I'd open code this inside the loop of cgroup_freeze since it is not
> context-less function and it relies on top-down processing.
> 
> Root cgrp cannot be frozen. You can bail out early in the beginning of
> cgroup_freeze() (possibly with a WARN_ON) and then assume parent is
> always valid when iterating.
> 

'cgroup.freeze' is with CFTYPE_NOT_ON_ROOT tag, root cgroup will never 
be frozen, considering root cgroup seems to be unesessary. I will open 
the code inside the loop of cgroup_freeze in next patch.

Thanks
Ridong

> I think maintaining the e_freeze in this "saturated arithmetic" form is
> a sensible change.
> 
> Thanks,
> Michal