Re: [RFC PATCH v1.1 04/11] mm/damon/sysfs-schemes: kobject_del() scheme region dirs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix: `damos_sysfs_probes_rm_dirs()` lacks `kobject_del()` during array updates.
- [High] Missing `kobject_del()` in error paths after successful `kobject_init_and_add()`.
--

commit 092044609406a531b0aee50bbd25500efab8e006
Author: SeongJae Park <sj@kernel.org>
Subject: mm/damon/sysfs-schemes: kobject_del() scheme region 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 region directories by adding kobject_del()
calls.

Link: https://lore.kernel.org/20260517205828.6204-1-sj@kernel.org

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 37cf6edb54f17..bf08e6e1f1635 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
> @@ -334,6 +334,7 @@ static void damon_sysfs_scheme_regions_rm_dirs(
>  	list_for_each_entry_safe(r, next, &regions->regions_list, list) {
>  		damos_sysfs_region_rm_dirs(r);

[Severity: High]
This is a pre-existing issue, but since damos_sysfs_region_rm_dirs()
calls damos_sysfs_probes_rm_dirs(), does damos_sysfs_probes_rm_dirs()
also need to explicitly call kobject_del() to avoid the same name
conflict regressions?

Looking at damos_sysfs_probes_rm_dirs():

mm/damon/sysfs-schemes.c:damos_sysfs_probes_rm_dirs() {
	...
	for (i = 0; i < probes->nr; i++)
		kobject_put(&probes_arr[i]->kobj);
	...
}

If kobject_del() isn't called here prior to kobject_put(), won't delayed
releases under CONFIG_DEBUG_KOBJECT_RELEASE collide with immediate
recreations, leading to -EEXIST failures when retrying probe creation?


[Severity: High]
This isn't a bug introduced by this patch, but does this code also miss
kobject_del() in the error paths after successful kobject_init_and_add()
calls?

In the error paths for directory creation, if kobject_init_and_add()
succeeds but subsequent setup steps fail, should the error rollback
explicitly call kobject_del() before kobject_put() to unlink the
directory?

mm/damon/sysfs-schemes.c:damos_sysfs_region_add_dirs() {
	...
	err = kobject_init_and_add(&probes->kobj, &damos_sysfs_probes_ktype,
			&region->kobj, "probes");
	if (err)
		goto fail;
	err = damos_sysfs_probes_add_dirs(probes, ctx, dregion);
	if (err)
		goto fail;
	...
fail:
	kobject_put(&probes->kobj);
	return err;
}

mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs() {
	...
		err = kobject_init_and_add(&scheme->kobj,
				&damon_sysfs_scheme_ktype, &schemes->kobj,
				"%d", i);
		if (err)
			goto out;
		err = damon_sysfs_scheme_add_dirs(scheme);
		if (err)
			goto out;
	...
out:
	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;
}

If kobject_del() is omitted in these error paths, could the directory
remain anchored in sysfs, causing -EEXIST name conflicts on retry
because the array count was not incremented?

>  		list_del(&r->list);
> +		kobject_del(&r->kobj);
>  		kobject_put(&r->kobj);
>  		regions->nr_regions--;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617144807.91441-1-sj@kernel.org?part=4