Re: [RFC PATCH v2.2 03/28] mm/damon/core: introduce damon_filter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Unsafe macro definitions for `damon_for_each_filter` and `damon_for_each_filter_safe`.
- [Low] Typo in the kernel-doc block for `struct damon_filter`.
--

commit 483481d8b5710ceb3aa6769a6360b2dc2305434c
Author: SeongJae Park <sj@kernel.org>
mm/damon/core: introduce damon_filter

This commit defines a new struct damon_filter and embeds it into
struct damon_probe for monitoring purposes. It also implements essential
handling and helpers such as filter allocation, addition, and freeing.

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 3ba5a31b51769..9c2576f103767 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
[ ... ]
> @@ -721,12 +721,38 @@ struct damon_intervals_goal {
>  	unsigned long max_sample_us;
>  };
>  
> +/**
> + * enum damon_filter_type - Type of &struct damon_filter
> + *
> + * @DAMON_FILTER_TYPE_ANON:	Anonymous pages.
> + */
> +enum damon_filter_type {
> +	DAMON_FILTER_TYPE_ANON,
> +};
> +
> +/**
> + * struct damon_filter - DAMON region filter for &struct damon_probe.
> + *
> + * @type:	Type of the region.
> + * @matcing:	Whether this filter is for the type-matching ones.

This isn't a bug, but there's a typo in the kernel-doc block here.
@matcing: should be @matching: to match the struct member name. This could
trigger kernel-doc warnings during compilation.

> + * @allow:	Whether the @type-@matching ones should pass this filter.
> + * @list:	Siblings list.
> + */
> +struct damon_filter {
> +	enum damon_filter_type type;
> +	bool matching;
> +	bool allow;
> +	struct list_head list;
> +};
[ ... ]
> @@ -880,6 +906,12 @@ static inline unsigned long damon_sz_region(struct damon_region *r)
>  	return r->ar.end - r->ar.start;
>  }
>  
> +#define damon_for_each_filter(f, p) \
> +	list_for_each_entry(f, &p->filters, list)
> +
> +#define damon_for_each_filter_safe(f, next, p) \
> +	list_for_each_entry_safe(f, next, &p->filters, list)

Is it possible this could cause operator precedence issues if a complex
expression or cast is passed for p?

It might be safer to wrap the macro argument p in parentheses, like
&(p)->filters, to avoid potential evaluation errors and compilation
failures in the future.

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