* Re: [PATCH 6.1.y] mm/damon/sysfs-schemes: kobject_del() scheme dirs
2026-09-09 3:55 ` [PATCH 6.1.y] mm/damon/sysfs-schemes: kobject_del() scheme dirs SJ Park
@ 2026-09-09 4:05 ` sashiko-bot
2026-09-09 20:26 ` Sasha Levin
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-09 4:05 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Incomplete fix: Missing `kobject_del()` for dynamically sized sysfs array directories (kdamonds, contexts, targets, regions) causes name conflicts on recreation.
--
commit 313d3e99fc72ef8df36e0924dc61e045a278961d
Author: SJ Park <sj@kernel.org>
mm/damon/sysfs-schemes: kobject_del() scheme dirs
On CONFIG_DEBUG_KOBJECT_RELEASE enabled kernel, lack of kobject_del()
could cause directories creation failures due to the name conflicts. Fix
those issues for scheme directories by adding kobject_del() calls.
Link: https://lore.kernel.org/20260628220121.97360-4-sj@kernel.org
> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index bb2f4650741fa..7e4fb112d1154 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -954,6 +954,7 @@ static void damon_sysfs_schemes_rm_dirs(struct damon_sysfs_schemes *schemes)
>
> for (i = 0; i < schemes->nr; i++) {
> damon_sysfs_scheme_rm_dirs(schemes_arr[i]);
> + kobject_del(&schemes_arr[i]->kobj);
> kobject_put(&schemes_arr[i]->kobj);
> }
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this fix for
the schemes directories, it appears the exact same omission of kobject_del()
is still present for other dynamically sized sysfs arrays (kdamonds,
contexts, targets, and regions).
For instance, in damon_sysfs_kdamonds_rm_dirs():
static void damon_sysfs_kdamonds_rm_dirs(struct damon_sysfs_kdamonds *kdamonds)
{
...
for (i = 0; i < kdamonds->nr; i++) {
damon_sysfs_kdamond_rm_dirs(kdamonds_arr[i]);
kobject_put(&kdamonds_arr[i]->kobj);
}
...
}
When updating these directories, the old directories are dropped using only
kobject_put() and new ones are immediately added with kobject_init_and_add()
using the same names. If CONFIG_DEBUG_KOBJECT_RELEASE is enabled or a
reference is held, won't this deferred removal leave the old directory
active and cause the recreation to fail with -EEXIST?
Should kobject_del() also be added to damon_sysfs_kdamonds_rm_dirs(),
damon_sysfs_contexts_rm_dirs(), damon_sysfs_targets_rm_dirs(), and
damon_sysfs_regions_rm_dirs() to prevent these same name conflicts?
> @@ -991,13 +992,15 @@ static int damon_sysfs_schemes_add_dirs(struct damon_sysfs_schemes *schemes,
> goto out;
> err = damon_sysfs_scheme_add_dirs(scheme);
> if (err)
> - goto out;
> + goto del_out;
>
> schemes_arr[i] = scheme;
> schemes->nr++;
> }
> return 0;
>
> +del_out:
> + kobject_del(&scheme->kobj);
> out:
> damon_sysfs_schemes_rm_dirs(schemes);
> kobject_put(&scheme->kobj);
[Severity: High]
This is also a pre-existing issue, but similar to the rm_dirs() paths, the
error paths for adding other array elements appear to be missing
kobject_del().
For example, in damon_sysfs_kdamonds_add_dirs():
for (i = 0; i < nr_kdamonds; i++) {
...
err = kobject_init_and_add(&kdamond->kobj,
&damon_sysfs_kdamond_ktype, &kdamonds->kobj,
"%d", i);
if (err)
goto out;
err = damon_sysfs_kdamond_add_dirs(kdamond);
if (err)
goto out;
...
out:
damon_sysfs_kdamonds_rm_dirs(kdamonds);
kobject_put(&kdamond->kobj);
return err;
If adding the child directories via damon_sysfs_kdamond_add_dirs() fails,
the parent kdamond->kobj has already been added to sysfs. Doesn't branching
directly to 'out' leave a lingering sysfs node without calling kobject_del()?
Should a similar 'del_out' error path be added to
damon_sysfs_kdamonds_add_dirs(), damon_sysfs_contexts_add_dirs(), and
damon_sysfs_targets_add_dirs()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260909035529.159422-1-sj@kernel.org?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread