Re: [RFC PATCH v2.2 12/17] mm/damon/sysfs: implement prep_action file

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `prep_action` configuration is completely ignored and never applied to the DAMON core.
--

--- Patch [12]: [RFC PATCH v2.2 12/17] mm/damon/sysfs: implement prep_action file ---
commit 5572380654e6a74889b64fc57510a982daf24d8e
Author: SJ Park <[email protected]>

mm/damon/sysfs: implement prep_action file

Add a file named prep_action under the prep directory.  It represents
the corresponding probe preparation action.

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index afd39659d6b84..c0130843223d7 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -756,6 +756,7 @@ static const struct kobj_type damon_sysfs_intervals_ktype = {
>  
>  struct damon_sysfs_prep {
>  	struct kobject kobj;
> +	enum damon_prep_action action;
>  };
>  
>  static struct damon_sysfs_prep *damon_sysfs_prep_alloc(void)

[ ... ]

> +static ssize_t prep_action_store(struct kobject *kobj,
> +		struct kobj_attribute *attr, const char *buf, size_t count)
> +{
> +	struct damon_sysfs_prep *prep = container_of(kobj,
> +			struct damon_sysfs_prep, kobj);
> +	ssize_t ret = -EINVAL;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(damon_sysfs_prep_action_names); i++) {
> +		const struct damon_sysfs_prep_action_name *action_name;
> +
> +		action_name = &damon_sysfs_prep_action_names[i];
> +		if (sysfs_streq(buf, action_name->name)) {
> +			prep->action = action_name->action;

[Severity: Medium]
Are we missing the logic to apply this configuration to the DAMON core?

Looking at damon_sysfs_set_probe(), it commits sysfs configurations to a probe
by calling damon_sysfs_set_filters(), but it appears to skip the preps
directory entirely:

static int damon_sysfs_set_probe(struct damon_probe *probe,
		struct damon_sysfs_probe *sys_probe)
{
	struct damon_sysfs_filters *sys_filters;

	sys_filters = sys_probe->filters;
	if (!sys_filters)
		return 0;
	return damon_sysfs_set_filters(probe, sys_filters);
}

Does this mean the prep_action configuration is never applied when a user
commits the context?

> +			ret = count;
> +			break;
> +		}
> +	}
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12