Re: [PATCH RFC 2/2] cgroup/dmem: add a node to double charge in memcg

["Michal Koutný" <mkoutny@suse.com>]

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

Hi.

On Fri, Apr 03, 2026 at 10:08:36AM -0400, Eric Chanudet <echanude@redhat.com> wrote:
> Introduce /cgroupfs/<>/dmem.memcg to make allocations in a dmem
> controlled region also be charged in memcg.
> 
> This is disabled by default and requires the administrator to configure
> it through the cgroupfs before the first charge occurs.

This somehow dropped the reason from [1] that this should be per-cgroup
controllable. Is that still valid? (Otherwise, I'd ask why not make this
a simple boot cmdline parameter like cgroup.memory=nokmem.)



> @@ -624,6 +656,13 @@ void dmem_cgroup_uncharge(struct dmem_cgroup_pool_state *pool, u64 size)
>  		return;
>  
>  	page_counter_uncharge(&pool->cnt, size);
> +
> +	struct mem_cgroup *memcg = mem_cgroup_from_cgroup(pool->cs->css.cgroup);

This is not necessarily same memcg as when the dmem was charged via
current (imagine dmem controller to depth N, but memcg only to N-1;
charge, then memcg is enabled up to N so this would attempt uncharge
from new memcg at level N, possibly going negative).

There is a question whether dmem should enforce same-depth hierarchies
with `dmem_cgrp_subsys.depends_on = 1 << memory_cgrp_id` (see
io_cgrp_subsys for comparison).

And eventually, if per-cgroup attribute is desired, it would make
greater sense to me if that attribute was on the parent level, so that
siblings competing among each other are always of the same composition
(i.e. all w/out dmem or all including dmem). This likely results in this
extra-charging attribute to be properly hierarchical.

HTH,
Michal

[1] https://lore.kernel.org/all/a446b598-5041-450b-aaa9-3c39a09ff6a0@amd.com/

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