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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit introduces `DAMON_FILTER_TYPE_PGIDLE_UNSET` but entirely omits its implementation in both the filter evaluation logic and the sysfs interface.
--

--- Patch [1]: [RFC PATCH 01/17] mm/damon/core: introduce DAMON_FILTER_TYPE_PGIDLE_UNSET ---
commit 227024f25ad71dec25a7b05a4270affaab5be023
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 the filter
evaluation logic?

Looking at damon_pa_filter_match() in mm/damon/paddr.c, the switch
statement evaluates filters but lacks a case to process the newly
introduced type:

mm/damon/paddr.c:damon_pa_filter_match() {
        ...
        switch (filter->type) {
        case DAMON_FILTER_TYPE_ANON:
        ...
}

Will this cause the evaluation to fall through to a default case and behave
incorrectly rather than matching the PG_Idle or accessed bit as intended?

Also, does this new type need to be mapped in the sysfs interface so userspace
can configure it? The damon_sysfs_filter_type_names array in mm/damon/sysfs.c
doesn't seem to include the new mapping:

mm/damon/sysfs.c: {
        ...
        static const struct damon_sysfs_filter_type_name
        damon_sysfs_filter_type_names[] = {
        ...
}

>  };
>  
>  /**

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