Re: [RFC PATCH v2 11/17] mm/damon/sysfs: implement probe prep directory

On Sun, 26 Jul 2026 11:57:15 -0700 SJ Park <[email protected]> wrote:

> On Sun, 26 Jul 2026 17:47:43 +0000 [email protected] wrote:
> 
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [Low] Missing `static` qualifier for `damon_sysfs_prep_ktype`.
> > - [Low] The `name` field in `struct damon_sysfs_prep_action_name` is declared as a non-const `char *` but initialized with read-only string literals.
> > --
> > 
> > --- Patch [11]: [RFC PATCH v2 11/17] mm/damon/sysfs: implement probe prep directory ---
> > commit ec46e0d8618f958d1edbf7e0edacd883d4e30bde
> > Author: SJ Park <[email protected]>
> > 
> > mm/damon/sysfs: implement probe prep directory
> > 
> > This commit implements the DAMON sysfs directory and files for specifying each
> > probe prep action. It introduces the preparatory data structures and functions,
> > though the files are not yet created under sysfs.
> > 
> > > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > > index 38affef08d0fb..a33423d997f59 100644
> > > --- a/mm/damon/sysfs.c
> > > +++ b/mm/damon/sysfs.c
> > > @@ -750,6 +750,89 @@ static const struct kobj_type damon_sysfs_intervals_ktype = {
> > 
> > [ ... ]
> > 
> > > +struct damon_sysfs_prep_action_name {
> > > +	enum damon_prep_action action;
> > > +	char *name;
> > 
> > [Severity: Low]
> > This isn't a bug, but should this field be a const char * instead of
> > char *? The damon_sysfs_prep_action_names array initializes it with
> > read-only string literals like "set_pgidle".
> 
> Good suggestion.  I will constify both fields.
> 
> > 
> > > +};
> > > +
> > > +static const struct damon_sysfs_prep_action_name
> > > +damon_sysfs_prep_action_names[] = {
> > > +	{
> > > +		.action = DAMON_PREP_SET_PGIDLE,
> > > +		.name = "set_pgidle",
> > > +	},
> > > +};
> > 
> > [ ... ]
> > 
> > > +static struct attribute *damon_sysfs_prep_attrs[] = {
> > > +	&damon_sysfs_prep_prep_action_attr.attr,
> > > +	NULL,
> > > +};
> > > +ATTRIBUTE_GROUPS(damon_sysfs_prep);
> > > +
> > > +const struct kobj_type damon_sysfs_prep_ktype = {
> > 
> > [Severity: Low]
> > This isn't a bug, but is the static qualifier missing here? Without it,
> > damon_sysfs_prep_ktype is exported globally which might trigger build
> > warnings like -Wmissing-prototypes when compiling with W=1.
> 
> It didn't warn on my setup.  Let me think more...

I still cannot make the warning on my setup.  But the theory makes sense.  I
will fix this by changing the patch sequence.

> 
> > 
> > > +	.release = damon_sysfs_prep_release,
> > > +	.sysfs_ops = &kobj_sysfs_ops,
> > > +	.default_groups = damon_sysfs_prep_groups,
> > > +};
> > 
> > -- 
> > Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
> 
> 
> Thanks,
> SJ
> 


Thanks,
SJ