On 17.01.25 18:29, Michal Koutný wrote:
On Thu, Jan 16, 2025 at 09:20:08AM +0100, Friedrich Vock <friedrich.vock@xxxxxx> wrote:
These pools are allocated on-demand, so if a
cgroup has not made any allocations for a specific device, there will be
no pool corresponding to that device's memory.
Here I understand.
Pools have a hierarchy of their own (that is, for a given cgroup's
pool corresponding to some device, the "parent pool" refers to the
parent cgroup's pool corresponding to the same device).
In dmem_cgroup_calculate_protection, we're trying to update the
protection values for the entire pool hierarchy between
limit_pool/test_pool (with the end goal of having accurate
effective-protection values for test_pool).
If you check and bail out at start:
if (!cgroup_is_descendant(test_pool->cs->css.cgroup, limit_pool->cs->css.cgroup))
return;
...
Since pools only store parent pointers to establish that hierarchy, to
find child pools given only the parent pool, we iterate over the pools
of all child cgroups and check if the parent pointer matches with our
current "parent pool" pointer.
The bug happens when some cgroup doesn't have any pool in the hierarchy
we're iterating over (that is, we iterate over all pools but don't find
any pool whose parent matches our current "parent pool" pointer).
...then the initial check ensures, you always find a pool that is
a descendant of limit_pool (at least the test_pool).
And there are pools for whole path between limit_pool and test_pool, or
am I mistaken here?
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.
The cgroup itself is part of the (cgroup) hierarchy, so the result of
cgroup_is_descendant is obviously true - but because of how we
allocate pools on-demand, it's still possible there is no pool that is
part of the (pool) hierarchy we're iterating over.
Can there be a pool without cgroup?
No, each pool is always associated with exactly one cgroup. What I was
talking about was the case where a parent cgroup has pools A and B, but
its child cgroup only has a pool for A. In that case, the child cgroup
has no pool that is part of B's pool hierarchy.
Thanks,
Friedrich
