Re: [PATCH 2/6] mm/damon/sysfs: implement update_schemes_quota_goals command

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent lockless modification of the sysfs current_value field causes a data race and silently destroys uncommitted user input.
--

commit 82969e621e89de8b04fc04548d5cd270bf1bb3b7
Author: Maksym Shcherba <mshcherba2000@gmail.com>

mm/damon/sysfs: implement update_schemes_quota_goals command

This commit introduces the DAMON_SYSFS_CMD_UPDATE_SCHEMES_QUOTA_GOALS
command. It adds the logic to copy the current_value from the internal
damos_quota_goal structure to the damos_sysfs_quota_goal sysfs structure
and integrates it with the sysfs interface.

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 622c3799db87b..198a510003143 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c

[ ... ]

> +void damos_sysfs_update_quota_goals(
> +		struct damon_sysfs_schemes *sysfs_schemes,
> +		struct damon_ctx *ctx)
> +{

[ ... ]

> +		damos_for_each_quota_goal(goal, &scheme->quota) {
> +			if (goals_idx >= sysfs_goals->nr)
> +				break;
> +
> +			sysfs_goals->goals_arr[goals_idx++]->current_value =
> +				goal->current_value;

Looking at current_value_store() in mm/damon/sysfs-schemes.c, user input is
written directly to the sysfs goal->current_value without holding
damon_sysfs_lock or using atomic operations.

Since the sysfs writer does not hold the lock, will this concurrent
modification create a data race that triggers KCSAN warnings?

> +		}
> +	}
> +}

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index d5863cc33d230..ecc880b52b329 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c

[ ... ]

> @@ -1656,6 +1672,7 @@ static int damon_sysfs_repeat_call_fn(void *data)
>  	damon_sysfs_upd_tuned_intervals(sysfs_kdamond);
>  	damon_sysfs_upd_schemes_stats(sysfs_kdamond);
>  	damon_sysfs_upd_schemes_effective_quotas(sysfs_kdamond);
> +	damon_sysfs_upd_schemes_quota_goals(sysfs_kdamond);
>  out:
>  	mutex_unlock(&damon_sysfs_lock);
>  	return 0;

Since damos_sysfs_update_quota_goals() is executed periodically here
(when refresh_ms > 0), the DAMON worker thread can overwrite a user's
uncommitted quota update with the old core state.

For goals using DAMOS_QUOTA_USER_INPUT, could this cause user input to be
lost before they have a chance to execute commit_schemes_quota_goals?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518190932.42270-1-maksym.shcherba@lnu.edu.ua?part=2