Re: [PATCH] cgroup/dmem: Don't clobber pool in dmem_cgroup_calculate_protection

Michal Koutný
2025-01-24T09:56:10Z

[-- Attachment #1: Type: text/plain, Size: 1652 bytes --]

On Fri, Jan 17, 2025 at 08:02:55PM +0100, Friedrich Vock <friedrich.vock@gmx.de> wrote:
> Yeah, there are pools for the whole path between limit_pool and
> test_pool, but the issue is that we traverse the entire tree of cgroups,
> and we don't always stay on the path between limit_pool and test_pool
> (because we're iterating from the top down, and we don't know what the
> path is in that direction - so we just traverse the whole tree until we
> find test_pool).
>
> This means that we'll sometimes end up straying off-path - and there are
> no guarantees for which pools are present in the cgroups we visit there.
> These cgroups are the potentially problematic ones where the issue can
> happen.
>
> Ideally we could always stay on the path between limit_pool and
> test_pool, but this is hardly possible because we can only follow parent
> links (so bottom-up traversal) but for accurate protection calculation
> we need to traverse the path top-down.

Aha, thanks for bearing with me.

css_foreach_descendant_pre(css, limit_pool->cs->css) {
dmemcg_iter = container_of(css, struct dmemcg_state, css);

struct dmem_cgroup_pool_state *found_pool = NULL;
list_for_each_entry_rcu(pool, &dmemcg_iter->pools, css_node) {
if (pool->region == limit_pool->region) {
found_pool = pool
break;
}
}
if (!found_pool)
continue;

page_counter_calculate_protection(
climit, &found->cnt, true);
}

Here I use (IMO) more idiomatic css_foreach_descendant_pre() instead and
I use the predicate based on ->region (correct?) to match pool's
devices.

Would that work as intended?

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]