Question on damon_sysfs_memcg_path_to_id() path resolution

Question on damon_sysfs_memcg_path_to_id() path resolution

[Song Hu]

Hi SeongJae,

I've been reading the memcg path->id resolution used by DAMOS filters
and quota goals, and I'd like to ask about a design choice before
suggesting any change.

damon_sysfs_memcg_path_to_id() (mm/damon/sysfs-common.c) resolves the
user-written cgroup path by iterating all memcgs with
mem_cgroup_iter() and comparing each one's cgroup_path() to the
target.  It could instead call cgroup_get_from_path() directly, but
the two don't resolve the path the same way:

  - current code: cgroup_path(memcg->css.cgroup) is just kernfs_path(),
    i.e. the absolute path in the cgroup hierarchy, independent of the
    caller's cgroup namespace;

  - cgroup_get_from_path(): resolves the path against
    current_cgns_cgroup_dfl(), i.e. relative to the caller's cgroup
    namespace.

So in a cgroup-namespaced (container) setup the two disagree: today
the lookup is absolute; switching to cgroup_get_from_path() would make
it namespace-relative.  On the host / init namespace they agree.

I ask because the iteration has already needed one stable fix
(d4e7b5c4cc35, a missing mem_cgroup_iter_break() that leaked a cgroup
reference), and cgroup_get_from_path() is a matched get/put pair that
would avoid that whole class of mistake -- but only if the namespace
behavior change is acceptable.

Is the absolute-path behavior intentional?  If namespace-relative
resolution is fine, I'm happy to send a patch (keeping the
mem_cgroup_online() skip).  If not, I'll leave it as is.

Thanks,
Song

Re: Question on damon_sysfs_memcg_path_to_id() path resolution

[SJ Park]

On Wed, 15 Jul 2026 14:35:46 +0800 Song Hu <husong@kylinos.cn> wrote:

> Hi SeongJae,
> 
> I've been reading the memcg path->id resolution used by DAMOS filters
> and quota goals, and I'd like to ask about a design choice before
> suggesting any change.

Thank you for asking this question!

> 
> damon_sysfs_memcg_path_to_id() (mm/damon/sysfs-common.c) resolves the
> user-written cgroup path by iterating all memcgs with
> mem_cgroup_iter() and comparing each one's cgroup_path() to the
> target.  It could instead call cgroup_get_from_path() directly, but
> the two don't resolve the path the same way:
> 
>   - current code: cgroup_path(memcg->css.cgroup) is just kernfs_path(),
>     i.e. the absolute path in the cgroup hierarchy, independent of the
>     caller's cgroup namespace;
> 
>   - cgroup_get_from_path(): resolves the path against
>     current_cgns_cgroup_dfl(), i.e. relative to the caller's cgroup
>     namespace.

Today I learned cgroup_get_from_path() :)

And it seems it will work for not only memcg but any cgroups?

> 
> So in a cgroup-namespaced (container) setup the two disagree: today
> the lookup is absolute; switching to cgroup_get_from_path() would make
> it namespace-relative.  On the host / init namespace they agree.

If my above guess is correct, it may also differently work if the user gives
non-memcg cgroup path.

> 
> I ask because the iteration has already needed one stable fix
> (d4e7b5c4cc35, a missing mem_cgroup_iter_break() that leaked a cgroup
> reference), and cgroup_get_from_path() is a matched get/put pair that
> would avoid that whole class of mistake -- but only if the namespace
> behavior change is acceptable.
> 
> Is the absolute-path behavior intentional?  If namespace-relative
> resolution is fine, I'm happy to send a patch (keeping the
> mem_cgroup_online() skip).  If not, I'll leave it as is.

It was not very intentional.  I just didn't think about container-inside DAMON
use case.

I find the current behavior might be tedious if the user runs DAMON inside
cgroups.  Finding the absolute cgroup path inside containers might be
challenging.  If that's the case, I'm up to change or extend the behaviors.

If this is only for making the code easier to maintain, I don't really feel
like it deserves the behavioral changes, to be honest.


Thanks,
SJ

[...]

Re: Question on damon_sysfs_memcg_path_to_id() path resolution

[Song Hu]

Hi,

Sorry for the late reply.

在 2026/7/15 21:51, SJ Park 写道:
> On Wed, 15 Jul 2026 14:35:46 +0800 Song Hu <husong@kylinos.cn> wrote:
>
>> Hi SeongJae,
>>
>> I've been reading the memcg path->id resolution used by DAMOS filters
>> and quota goals, and I'd like to ask about a design choice before
>> suggesting any change.
> Thank you for asking this question!
>
>> damon_sysfs_memcg_path_to_id() (mm/damon/sysfs-common.c) resolves the
>> user-written cgroup path by iterating all memcgs with
>> mem_cgroup_iter() and comparing each one's cgroup_path() to the
>> target.  It could instead call cgroup_get_from_path() directly, but
>> the two don't resolve the path the same way:
>>
>>   - current code: cgroup_path(memcg->css.cgroup) is just kernfs_path(),
>>     i.e. the absolute path in the cgroup hierarchy, independent of the
>>     caller's cgroup namespace;
>>
>>   - cgroup_get_from_path(): resolves the path against
>>     current_cgns_cgroup_dfl(), i.e. relative to the caller's cgroup
>>     namespace.
> Today I learned cgroup_get_from_path() :)
>
> And it seems it will work for not only memcg but any cgroups?
>
>> So in a cgroup-namespaced (container) setup the two disagree: today
>> the lookup is absolute; switching to cgroup_get_from_path() would make
>> it namespace-relative.  On the host / init namespace they agree.
> If my above guess is correct, it may also differently work if the user gives
> non-memcg cgroup path.
>
>> I ask because the iteration has already needed one stable fix
>> (d4e7b5c4cc35, a missing mem_cgroup_iter_break() that leaked a cgroup
>> reference), and cgroup_get_from_path() is a matched get/put pair that
>> would avoid that whole class of mistake -- but only if the namespace
>> behavior change is acceptable.
>>
>> Is the absolute-path behavior intentional?  If namespace-relative
>> resolution is fine, I'm happy to send a patch (keeping the
>> mem_cgroup_online() skip).  If not, I'll leave it as is.
> It was not very intentional.  I just didn't think about container-inside DAMON
> use case.
>
> I find the current behavior might be tedious if the user runs DAMON inside
> cgroups.  Finding the absolute cgroup path inside containers might be
> challenging.  If that's the case, I'm up to change or extend the behaviors.
>
> If this is only for making the code easier to maintain, I don't really feel
> like it deserves the behavioral changes, to be honest.
I looked into the container case more carefully and I'm no longer sure
it justifies the change.

DAMON sysfs is root-only (state is 0600 under /sys/kernel/mm/damon), so
whoever configures a scheme is effectively a host-level operator.  That
operator can obtain the absolute cgroup path directly (e.g. from
/proc/<pid>/cgroup), and the usage docs already show host-absolute
paths.  A process able to write host DAMON sysfs from inside a non-init
cgroup namespace is a privileged host-admin case, not really a "DAMON
inside a container" user.  So I think the namespace-relative benefit is
marginal at best.

Given that, and your point that maintainability alone isn't worth a
behavioral change, I'll leave damon_sysfs_memcg_path_to_id() as-is.

Thanks for thinking it through with me -- and for the cgroup_get_from_path pointer.

Song
>
> Thanks,
> SJ
>
> [...]

Re: Question on damon_sysfs_memcg_path_to_id() path resolution

[SJ Park]

On Sat, 18 Jul 2026 16:40:51 +0800 Song Hu <husong@kylinos.cn> wrote:

> Hi,
> 
> Sorry for the late reply.

No worry!

[...]
> > I find the current behavior might be tedious if the user runs DAMON inside
> > cgroups.  Finding the absolute cgroup path inside containers might be
> > challenging.  If that's the case, I'm up to change or extend the behaviors.
> >
> > If this is only for making the code easier to maintain, I don't really feel
> > like it deserves the behavioral changes, to be honest.
> I looked into the container case more carefully and I'm no longer sure
> it justifies the change.
> 
> DAMON sysfs is root-only (state is 0600 under /sys/kernel/mm/damon), so
> whoever configures a scheme is effectively a host-level operator.  That
> operator can obtain the absolute cgroup path directly (e.g. from
> /proc/<pid>/cgroup), and the usage docs already show host-absolute
> paths.  A process able to write host DAMON sysfs from inside a non-init
> cgroup namespace is a privileged host-admin case, not really a "DAMON
> inside a container" user.  So I think the namespace-relative benefit is
> marginal at best.
> 
> Given that, and your point that maintainability alone isn't worth a
> behavioral change, I'll leave damon_sysfs_memcg_path_to_id() as-is.

Makes sense.

> 
> Thanks for thinking it through with me -- and for the cgroup_get_from_path pointer.

You're welcome.  I liked this dicsussion.  Looking forward to the next one!


Thanks,
SJ

[...]

Re: Question on damon_sysfs_memcg_path_to_id() path resolution

[Song Hu]

在 2026/7/18 23:59, SJ Park 写道:
> On Sat, 18 Jul 2026 16:40:51 +0800 Song Hu <husong@kylinos.cn> wrote:
>
>> Hi,
>>
>> Sorry for the late reply.
> No worry!
>
> [...]
>>> I find the current behavior might be tedious if the user runs DAMON inside
>>> cgroups.  Finding the absolute cgroup path inside containers might be
>>> challenging.  If that's the case, I'm up to change or extend the behaviors.
>>>
>>> If this is only for making the code easier to maintain, I don't really feel
>>> like it deserves the behavioral changes, to be honest.
>> I looked into the container case more carefully and I'm no longer sure
>> it justifies the change.
>>
>> DAMON sysfs is root-only (state is 0600 under /sys/kernel/mm/damon), so
>> whoever configures a scheme is effectively a host-level operator.  That
>> operator can obtain the absolute cgroup path directly (e.g. from
>> /proc/<pid>/cgroup), and the usage docs already show host-absolute
>> paths.  A process able to write host DAMON sysfs from inside a non-init
>> cgroup namespace is a privileged host-admin case, not really a "DAMON
>> inside a container" user.  So I think the namespace-relative benefit is
>> marginal at best.
>>
>> Given that, and your point that maintainability alone isn't worth a
>> behavioral change, I'll leave damon_sysfs_memcg_path_to_id() as-is.
> Makes sense.
>
>> Thanks for thinking it through with me -- and for the cgroup_get_from_path pointer.
> You're welcome.  I liked this dicsussion.  Looking forward to the next one!
Following up on our last chat - thanks again for the cgroup_get_from_path
pointer, that settled it for me. One thing it got me thinking about, which
I'd like to get your read on before I write any code.

The only in-kernel PSI consumer today is DAMON, and it reads a single
system-wide value - psi_system.total for memory (mm/damon/core.c,
damos_get_some_mem_psi_total). There is no reusable in-kernel helper that
returns per-cgroup, multi-domain (cpu/mem/io x some/full) pressure as a
value (psi_show is a seq_file printer for the /proc and cgroup files, not a
value-returning API), and no in-kernel threshold-trigger (psi_trigger_create
is userspace-fd only).

I'm writing a node-level multi-domain (cpu + memory + io) resource
coordinator for container/Kubernetes nodes - a small C daemon using libbpf
plus PSI trigger fds. To attribute pressure to specific cgroups and act
across domains it needs per-cgroup, per-domain PSI, which today means
open-coding reads of cgroup_psi(cgrp)->total[] the way DAMON does in-kernel.
And I think DAMON's own per-memcg DAMOS quota goals would benefit from the
same per-cgroup read if it existed.

So the idea is a small helper, e.g.

  /* per-cgroup, per-domain pressure; reuses update_averages() */
  u64 psi_cgroup_total(struct cgroup *cgrp, enum psi_res res, bool some);
  /* and/or an avg10/60/300 variant */

with DAMON migrated to it as the first in-tree user.

Before I do anything: does this capability gap resonate with you - is
per-cgroup PSI consumption something you'd want DAMON to grow into, or do
you see it staying userspace-only? And if a helper makes sense, any
preferred shape, or concerns about exposing the averaging/locking contract?
I'd keep the first cut to just the read helper + DAMON migration; an
in-kernel trigger callback I'd treat as a separate, later step and only if
there's a second consumer.

Thanks,
Song
>
> Thanks,
> SJ
>
> [...]

Re: Question on damon_sysfs_memcg_path_to_id() path resolution

[SJ Park]

On Mon, 20 Jul 2026 11:35:06 +0800 Song Hu <husong@kylinos.cn> wrote:

[...]
> The only in-kernel PSI consumer today is DAMON, and it reads a single
> system-wide value - psi_system.total for memory (mm/damon/core.c,
> damos_get_some_mem_psi_total). There is no reusable in-kernel helper that
> returns per-cgroup, multi-domain (cpu/mem/io x some/full) pressure as a
> value (psi_show is a seq_file printer for the /proc and cgroup files, not a
> value-returning API), and no in-kernel threshold-trigger (psi_trigger_create
> is userspace-fd only).
> 
> I'm writing a node-level multi-domain (cpu + memory + io) resource
> coordinator for container/Kubernetes nodes - a small C daemon using libbpf
> plus PSI trigger fds. To attribute pressure to specific cgroups and act
> across domains it needs per-cgroup, per-domain PSI,

Sounds interesting!

> which today means
> open-coding reads of cgroup_psi(cgrp)->total[] the way DAMON does in-kernel.
> And I think DAMON's own per-memcg DAMOS quota goals would benefit from the
> same per-cgroup read if it existed.
> 
> So the idea is a small helper, e.g.
> 
>   /* per-cgroup, per-domain pressure; reuses update_averages() */
>   u64 psi_cgroup_total(struct cgroup *cgrp, enum psi_res res, bool some);
>   /* and/or an avg10/60/300 variant */
> 
> with DAMON migrated to it as the first in-tree user.

I'm sorry but I'm missing your points from here.  Could you elaborate why we
need the helper, and why DAMON should use it?

You're implementing your system in user-space, so I don't think you will use
that helper for your system.  DAMON has its internal implementation for reading
PSI.  What's the benefit of the helper compared to the current one?

Also that helper may be implemented inside PSI or cgroup subsystem and
maintained by the maintainers of the subsystems.  Are they willing to have this
and increase their maintenance burden?

> 
> Before I do anything: does this capability gap resonate with you - is
> per-cgroup PSI consumption something you'd want DAMON to grow into, or do
> you see it staying userspace-only? And if a helper makes sense, any
> preferred shape, or concerns about exposing the averaging/locking contract?
> I'd keep the first cut to just the read helper + DAMON migration; an
> in-kernel trigger callback I'd treat as a separate, later step and only if
> there's a second consumer.

Seems you have larger picture.  I don't clearly show what is it.  Could you
share it at a high level?  That might enlighten me.


Thanks,
SJ

[...]