Re: [RFC PATCH 12/27] mm/damon/core: introduce DAMON_FILTER_TYPE_REPORT_MADE

[SJ Park <sj@kernel.org>]

On Sun, 15 Nov 2026 20:36:01 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [Medium] mm/damon: missing report_type copy in damon_commit_filters()
> - [Low] mm/damon: missing kernel-doc for report_type in struct
>   damon_filter
> --
> 
> commit 4743cf0114c7407b0d8b853557595a3a4674cb82
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: introduce DAMON_FILTER_TYPE_REPORT_MADE
> 
> This commit introduces the DAMON_FILTER_TYPE_REPORT_MADE probe filter
> type, adds report_type to struct damon_filter, and copies it in
> damon_commit_filter().
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index ca0135c6f6700..cce7d344c585b 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> 
> [ ... ]
> 
> > @@ -834,6 +836,7 @@ struct damon_filter {
> >  			unsigned long range_min;
> >  			unsigned long range_max;
> >  		};
> > +		enum damon_report_type report_type;
> >  	};
> 
> [Severity: Low]
> Should @report_type be documented in the kernel-doc comment for struct
> damon_filter to avoid a kernel-doc warning?

Nice catch.  I will add the kernel-doc comment.

> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 06b85d00e38d9..c02c7fbc1d864 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1834,6 +1834,9 @@ static int damon_commit_filter(struct damon_filter *dst,
> >  		dst->range_min = src->range_min;
> >  		dst->range_max = src->range_max;
> >  		break;
> > +	case DAMON_FILTER_TYPE_REPORT_MADE:
> > +		dst->report_type = src->report_type;
> > +		break;
> 
> [Severity: Medium]
> Does damon_commit_filters() also need to copy report_type when allocating
> a new filter?
> 
> damon_commit_filter() is only called when overwriting an existing filter
> in dst.  When src has more filters than dst, damon_commit_filters()
> allocates new_filter via damon_new_filter() (which uses kmalloc_obj()
> without initializing the union) and runs a separate switch on
> src_filter->type:
> 
> mm/damon/core.c:damon_commit_filters() {
> 	...
> 		new_filter = damon_new_filter(
> 				src_filter->type, src_filter->matching,
> 				src_filter->allow);
> 		if (!new_filter)
> 			return -ENOMEM;
> 		switch (src_filter->type) {
> 		case DAMON_FILTER_TYPE_MEMCG:
> 			new_filter->memcg_id = src_filter->memcg_id;
> 			break;
> 		case DAMON_FILTER_TYPE_HUGEPAGE_SIZE:
> 			...
> 			break;
> 		default:
> 			break;
> 		}
> 	...
> }
> 
> Because DAMON_FILTER_TYPE_REPORT_MADE is not handled there, committing a
> newly added report_made filter leaves new_filter->report_type
> uninitialized.

Good catch.  I will add that.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261115202849.99134-1-sj@kernel.org?part=12
> 


Thanks,
SJ