On 8/4/26 10:25, Natalie Vock wrote:
> This helps to find a common subtree of two resources, which is important
> when determining whether it's helpful to evict one resource in favor of
> another.
>
> To facilitate this, add a common helper to find the ancestor of two
> cgroups using each cgroup's ancestor array.
>
> Signed-off-by: Natalie Vock <[email protected]>
> ---
> include/linux/cgroup.h | 21 +++++++++++++++++++++
> include/linux/cgroup_dmem.h | 9 +++++++++
> kernel/cgroup/dmem.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 71 insertions(+)
>
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index f2aa46a4f871e..83a17ded1c516 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -623,6 +623,27 @@ static inline struct cgroup *cgroup_ancestor(struct
> cgroup *cgrp,
> return cgrp->ancestors[ancestor_level];
> }
>
> +/**
> + * cgroup_common_ancestor - find common ancestor of two cgroups
> + * @a: first cgroup to find common ancestor of
> + * @b: second cgroup to find common ancestor of
> + *
> + * Find the first cgroup that is an ancestor of both @a and @b, if it exists
> + * and return a pointer to it. If such a cgroup doesn't exist, return NULL.
> + *
> + * This function is safe to call as long as both @a and @b are accessible.
> + */
> +static inline struct cgroup *cgroup_common_ancestor(struct cgroup *a,
> + struct cgroup *b)
> +{
> + int level;
> +
> + for (level = min(a->level, b->level); level >= 0; level--)
> + if (a->ancestors[level] == b->ancestors[level])
> + return a->ancestors[level];
> + return NULL;
> +}
> +
> /**
> * task_under_cgroup_hierarchy - test task's membership of cgroup ancestry
> * @task: the task to be tested
> diff --git a/include/linux/cgroup_dmem.h b/include/linux/cgroup_dmem.h
> index 1a88cd0c9eb00..9d72457c4cb9d 100644
> --- a/include/linux/cgroup_dmem.h
> +++ b/include/linux/cgroup_dmem.h
> @@ -28,6 +28,8 @@ bool dmem_cgroup_below_min(struct dmem_cgroup_pool_state
> *root,
> struct dmem_cgroup_pool_state *test);
> bool dmem_cgroup_below_low(struct dmem_cgroup_pool_state *root,
> struct dmem_cgroup_pool_state *test);
> +struct dmem_cgroup_pool_state *dmem_cgroup_get_common_ancestor(struct
> dmem_cgroup_pool_state *a,
> + struct
> dmem_cgroup_pool_state *b);
>
> void dmem_cgroup_pool_state_put(struct dmem_cgroup_pool_state *pool);
> #else
> @@ -75,6 +77,13 @@ static inline bool dmem_cgroup_below_low(struct
> dmem_cgroup_pool_state *root,
> return false;
> }
>
> +static inline
> +struct dmem_cgroup_pool_state *dmem_cgroup_get_common_ancestor(struct
> dmem_cgroup_pool_state *a,
> + struct
> dmem_cgroup_pool_state *b)
> +{
> + return NULL;
> +}
> +
> static inline void dmem_cgroup_pool_state_put(struct dmem_cgroup_pool_state
> *pool)
> { }
>
> diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
> index 9df3b33c65046..a587611ca2235 100644
> --- a/kernel/cgroup/dmem.c
> +++ b/kernel/cgroup/dmem.c
> @@ -762,6 +762,47 @@ bool dmem_cgroup_below_low(struct dmem_cgroup_pool_state
> *root,
> }
> EXPORT_SYMBOL_GPL(dmem_cgroup_below_low);
>
> +/**
> + * dmem_cgroup_get_common_ancestor(): Find the first common ancestor of two
> pools.
> + * @a: First pool to find the common ancestor of.
> + * @b: First pool to find the common ancestor of.
> + *
> + * Return: The first pool that is a parent of both @a and @b, or NULL if
> either @a or @b are NULL,
> + * or if such a pool does not exist. A reference to the returned pool is
> grabbed and must be
> + * released by the caller when it is done using the pool.
> + */
> +struct dmem_cgroup_pool_state *dmem_cgroup_get_common_ancestor(struct
> dmem_cgroup_pool_state *a,
> + struct
> dmem_cgroup_pool_state *b)
> +{
> + struct cgroup *ancestor_cgroup;
> + struct cgroup_subsys_state *ancestor_css;
> + struct dmemcg_state *ancestor_dmemcs = NULL;
> + struct dmem_cgroup_pool_state *pool = NULL;
> +
> + if (!a || !b)
> + return NULL;
> +
> + ancestor_cgroup = cgroup_common_ancestor(a->cs->css.cgroup,
> b->cs->css.cgroup);
> + if (!ancestor_cgroup)
> + return NULL;
> +
> + rcu_read_lock();
> + ancestor_css = cgroup_e_css(ancestor_cgroup, &dmem_cgrp_subsys);
> + if (css_tryget(ancestor_css))
> + ancestor_dmemcs = css_to_dmemcs(ancestor_css);
> + rcu_read_unlock();
> +
> + if (ancestor_dmemcs) {
> + pool = get_cg_pool_unlocked(css_to_dmemcs(ancestor_css),
> + a->region);
> + if (IS_ERR(pool))
> + pool = NULL;
This should probably be a warn_on as this can never happen, sashiko is wrong
here.
If a and b are charged, all their ancestors are charged too. And when they
share a
common ancestor, then the common ancestor has to have an existing charged pool
as
well because there's a huge bug in the code otherwise.
> + css_put(ancestor_css);
> + }
> + return pool;
> +}
> +EXPORT_SYMBOL_GPL(dmem_cgroup_get_common_ancestor);
> +
> static int dmem_cgroup_region_capacity_show(struct seq_file *sf, void *v)
> {
> struct dmem_cgroup_region *region;
>
Otherwise looks good, so feel free to extend my r-b to this patch too, and with
that minor fix up commit it.
Reviewed-by: Maarten Lankhorst <[email protected]>
