Re: Question on damon_sysfs_memcg_path_to_id() path resolution

Hi,

Sorry for the late reply.

在 2026/7/15 21:51, SJ Park 写道:
> On Wed, 15 Jul 2026 14:35:46 +0800 Song Hu <[email protected]> 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
>
> [...]