Re: [RFC PATCH v2.1 3/7] mm/damon/core: support probe_hits_wsum damos core filter

[SJ Park <sj@kernel.org>]

On Thu, 10 Sep 2026 04:16:47 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] Missing kunit test coverage for the newly added DAMOS_FILTER_TYPE_PROBE_HITS_WSUM filter.
> - [Low] The kerneldoc for `enum damos_filter_type` was not updated to reflect that `DAMOS_FILTER_TYPE_PROBE_HITS_WSUM` is a core layer filter.
> --
> 
> commit 3af974fc20158a639272fd766ad727c8d30c0f3e
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: support probe_hits_wsum damos core filter
> 
> Implement probe_hits_wsum DAMOS core filter support in the core layer.
> Make three small changes for the support: update damos_filter_for_ops() to
> treat probe_hits_wsum filter as core filter, update destination
> damos_filter->range_{min,max} for probe_hits_wsum, and extend
> damos_filter_match() to handle the new filter type.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index e9630f41e51fe..d9d56f4be1a97 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -658,6 +658,7 @@ bool damos_filter_for_ops(enum damos_filter_type type)
> >  	switch (type) {
> >  	case DAMOS_FILTER_TYPE_ADDR:
> >  	case DAMOS_FILTER_TYPE_TARGET:
> > +	case DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:
> 
> [Severity: Low]
> Does the kerneldoc for enum damos_filter_type need to be updated to reflect
> that DAMOS_FILTER_TYPE_PROBE_HITS_WSUM is a core layer filter?
> 
> include/linux/damon.h:enum damos_filter_type {
>  * All types except &DAMOS_FILTER_TYPE_ADDR and &DAMOS_FILTER_TYPE_TARGET
>  * are handled by the underlying &struct damon_operations as a part of scheme
>  * action trying, and therefore accounted as 'tried'.
> 
> The documentation here explicitly lists only ADDR and TARGET as exceptions,
> failing to update the list with the newly added PROBE_HITS_WSUM core filter.

Yes, I will update so in the next revision.

> 
> >  		return false;
> >  	default:
> >  		break;
> 
> [ ... ]
> 
> > @@ -1332,6 +1333,10 @@ static void damos_commit_filter_arg(
> >  	case DAMOS_FILTER_TYPE_HUGEPAGE_SIZE:
> >  		dst->sz_range = src->sz_range;
> >  		break;
> > +	case DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:
> > +		dst->range_min = src->range_min;
> > +		dst->range_max = src->range_max;
> > +		break;
> 
> [Severity: Low]
> Is there missing KUnit test coverage for the newly added
> DAMOS_FILTER_TYPE_PROBE_HITS_WSUM filter?
> 
> The copying logic for the new filter type's arguments (range_min, range_max)
> is left untested, which could hide future regressions.
> 
> In mm/damon/tests/core-kunit.h:damos_test_commit_filter_for(), the switch
> statement verifies filter argument copying but is missing the case for
> DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:
> 
> 	case DAMOS_FILTER_TYPE_HUGEPAGE_SIZE:
> 		KUNIT_EXPECT_EQ(test, dst->sz_range.min, src->sz_range.min);
> 		KUNIT_EXPECT_EQ(test, dst->sz_range.max, src->sz_range.max);
> 		break;
> 	default:
> 		break;
> 	}
> 
> Additionally, in mm/damon/tests/core-kunit.h:damos_test_commit_filter(),
> the function enumerates all filter types for testing but fails to include
> instances of DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:
> 
> 	damos_test_commit_filter_for(test, &dst,
> 			&(struct damos_filter){
> 			.type = DAMOS_FILTER_TYPE_TARGET,
> 			.matching = true,
> 			.allow = true,
> 			.target_idx = 6,
> 			});

Good callout.  But definitely not a blocker of this series.  I will consider
adding such tests as a followup.

> 
> >  	default:
> >  		break;
> >  	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260910040319.147037-1-sj@kernel.org?part=3
> 


Thanks,
SJ