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

[Michal Koutný]

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

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.

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

Thanks,
Michal

Attachment: signature.asc
Description: PGP signature