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

[sashiko-bot@kernel.org]

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?

> 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.

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