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

[Chen Ridong]

On 2024/9/10 3:01, Michal Koutný wrote:
> On Thu, Sep 05, 2024 at 01:41:28PM GMT, Chen Ridong <chenridong@xxxxxxxxxx> 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