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

On Tue, 28 Jul 2026 04:23:45 +0000 [email protected] wrote:

> 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?

A later patch of this series does that.

> 
> > +			ret = count;
> > +			break;
> > +		}
> > +	}
> > +	return ret;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12


Thanks,
SJ