Re: [RFC PATCH v2.2 03/28] mm/damon/core: introduce damon_filter

[SeongJae Park <sj@kernel.org>]

On Fri, 15 May 2026 01:14:13 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] Unsafe macro definitions for `damon_for_each_filter` and `damon_for_each_filter_safe`.
> - [Low] Typo in the kernel-doc block for `struct damon_filter`.
> --
> 
> commit 483481d8b5710ceb3aa6769a6360b2dc2305434c
> Author: SeongJae Park <sj@kernel.org>
> mm/damon/core: introduce damon_filter
> 
> This commit defines a new struct damon_filter and embeds it into
> struct damon_probe for monitoring purposes. It also implements essential
> handling and helpers such as filter allocation, addition, and freeing.
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index 3ba5a31b51769..9c2576f103767 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> [ ... ]
> > @@ -721,12 +721,38 @@ struct damon_intervals_goal {
> >  	unsigned long max_sample_us;
> >  };
> >  
> > +/**
> > + * enum damon_filter_type - Type of &struct damon_filter
> > + *
> > + * @DAMON_FILTER_TYPE_ANON:	Anonymous pages.
> > + */
> > +enum damon_filter_type {
> > +	DAMON_FILTER_TYPE_ANON,
> > +};
> > +
> > +/**
> > + * struct damon_filter - DAMON region filter for &struct damon_probe.
> > + *
> > + * @type:	Type of the region.
> > + * @matcing:	Whether this filter is for the type-matching ones.
> 
> This isn't a bug, but there's a typo in the kernel-doc block here.
> @matcing: should be @matching: to match the struct member name. This could
> trigger kernel-doc warnings during compilation.

Good eyes!  I will fix.

> 
> > + * @allow:	Whether the @type-@matching ones should pass this filter.
> > + * @list:	Siblings list.
> > + */
> > +struct damon_filter {
> > +	enum damon_filter_type type;
> > +	bool matching;
> > +	bool allow;
> > +	struct list_head list;
> > +};
> [ ... ]
> > @@ -880,6 +906,12 @@ static inline unsigned long damon_sz_region(struct damon_region *r)
> >  	return r->ar.end - r->ar.start;
> >  }
> >  
> > +#define damon_for_each_filter(f, p) \
> > +	list_for_each_entry(f, &p->filters, list)
> > +
> > +#define damon_for_each_filter_safe(f, next, p) \
> > +	list_for_each_entry_safe(f, next, &p->filters, list)
> 
> Is it possible this could cause operator precedence issues if a complex
> expression or cast is passed for p?
> 
> It might be safer to wrap the macro argument p in parentheses, like
> &(p)->filters, to avoid potential evaluation errors and compilation
> failures in the future.

Nice suggestion, I will do so in the next revision.


Thanks,
SJ

[...]