Re: [RFC PATCH v2.2 01/17] mm/damon/core: introduce DAMON_FILTER_TYPE_PGIDLE_UNSET

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

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly introduced DAMON_FILTER_TYPE_PGIDLE_UNSET enum is unhandled in the filter matching logic and sysfs interface, leaving the promised functionality completely missing.
> --
> 
> --- Patch [1]: [RFC PATCH v2.2 01/17] mm/damon/core: introduce DAMON_FILTER_TYPE_PGIDLE_UNSET ---
> commit 1ede3c3f87a9caeeb7ad80090cbb84d20b477505
> Author: SJ Park <[email protected]>
> 
> mm/damon/core: introduce DAMON_FILTER_TYPE_PGIDLE_UNSET
>     
> Introduce a new DAMON filter type, pgidle_unset. It will match pages
> that have their PG_Idle flag unset, or the page table accessed bit set.
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index 0c8b7ddef9abb..a8bb9ee6979e3 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> > @@ -742,12 +742,14 @@ struct damon_intervals_goal {
> >  /**
> >   * enum damon_filter_type - Type of &struct damon_filter
> >   *
> > - * @DAMON_FILTER_TYPE_ANON:	Anonymous pages.
> > - * @DAMON_FILTER_TYPE_MEMCG:	Specific memcg's pages.
> > + * @DAMON_FILTER_TYPE_ANON:		Anonymous pages.
> > + * @DAMON_FILTER_TYPE_MEMCG:		Specific memcg's pages.
> > + * @DAMON_FILTER_TYPE_PGIDLE_UNSET:	Pgidle is unset.
> >   */
> >  enum damon_filter_type {
> >  	DAMON_FILTER_TYPE_ANON,
> >  	DAMON_FILTER_TYPE_MEMCG,
> > +	DAMON_FILTER_TYPE_PGIDLE_UNSET,
> 
> [Severity: Medium]
> Is the implementation for this new filter type missing from this patch?
> 
> Looking at damon_pa_filter_match() in mm/damon/paddr.c, the switch
> statement evaluating filter types does not appear to handle
> DAMON_FILTER_TYPE_PGIDLE_UNSET. This means it will fall through to the
> default case and fail to implement the promised page matching logic:
> 
> mm/damon/paddr.c:damon_pa_filter_match() {
>     ...
>     switch (filter->type) {
>     case DAMON_FILTER_TYPE_ANON:
>     ...
>     }
> }
> 
> Additionally, it looks like this new filter type is missing from the sysfs 
> name mapping array, damon_sysfs_filter_type_names, in mm/damon/sysfs.c:
> 
> mm/damon/sysfs.c:damon_sysfs_filter_type_names {
>     {
>         .type = DAMON_FILTER_TYPE_ANON,
>         .name = "anon",
>     },
>     ...
> }
> 
> Without this mapping, type_store() will fail with -EINVAL, making it 
> impossible for userspace to configure.
> 
> Will a follow-up patch add the handling logic and sysfs bindings, or should 
> they be included directly in this commit?

A later patch if this series does that.

> 
> >  };
> >  
> >  /**
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1


Thanks,
SJ