Re: [RFC PATCH v2 3/8] mm/damon/core: return an error from damos_commit_filter_arg()

[Kunwu Chan <kunwu.chan@gmail.com>]

On Sun, 13 Sep 2026 10:17:00 -0700 SJ Park <sj@kernel.org> wrote:

> damos_commit_filter_arg() is supposed to always succeed.  It may not in
> future, for example, if the given filter is invalid.  Prepare the case
> by modifying its signature to return an error when it failed.  Also pipe
> the return value to its callers and let them handle the error.
> 
> Signed-off-by: SJ Park <sj@kernel.org>
> ---
>  mm/damon/core.c | 41 ++++++++++++++++++++++++++++-------------
>  1 file changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 3d7a973ea8b6f..ea7d8d5a4abf6 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1317,7 +1317,7 @@ static struct damos_filter *damos_nth_ops_filter(int n, struct damos *s)
>  	return NULL;
>  }
>  
> -static void damos_commit_filter_arg(
> +static int damos_commit_filter_arg(
>  		struct damos_filter *dst, struct damos_filter *src)
>  {
>  	switch (dst->type) {
> @@ -1340,28 +1340,32 @@ static void damos_commit_filter_arg(
>  	default:
>  		break;
>  	}
> +	return 0;
>  }
>  
> -static void damos_commit_filter(
> +static int damos_commit_filter(
>  		struct damos_filter *dst, struct damos_filter *src)
>  {
>  	dst->type = src->type;
>  	dst->matching = src->matching;
>  	dst->allow = src->allow;
> -	damos_commit_filter_arg(dst, src);
> +	return damos_commit_filter_arg(dst, src);
>  }
>  
>  static int damos_commit_core_filters(struct damos *dst, struct damos *src)
>  {
>  	struct damos_filter *dst_filter, *next, *src_filter, *new_filter;
> -	int i = 0, j = 0;
> +	int i = 0, j = 0, err;
>  
>  	damos_for_each_core_filter_safe(dst_filter, next, dst) {
>  		src_filter = damos_nth_core_filter(i++, src);
> -		if (src_filter)
> -			damos_commit_filter(dst_filter, src_filter);
> -		else
> +		if (src_filter) {
> +			err = damos_commit_filter(dst_filter, src_filter);
> +			if (err)
> +				return err;
> +		} else {
>  			damos_destroy_filter(dst_filter);
> +		}
>  	}
>  
>  	damos_for_each_core_filter_safe(src_filter, next, src) {
> @@ -1373,7 +1377,11 @@ static int damos_commit_core_filters(struct damos *dst, struct damos *src)
>  				src_filter->allow);
>  		if (!new_filter)
>  			return -ENOMEM;
> -		damos_commit_filter_arg(new_filter, src_filter);
> +		err = damos_commit_filter_arg(new_filter, src_filter);
> +		if (err) {
> +			damos_destroy_filter(new_filter);
> +			return err;
> +		}
>  		damos_add_filter(dst, new_filter);
>  	}
>  	return 0;
> @@ -1382,14 +1390,17 @@ static int damos_commit_core_filters(struct damos *dst, struct damos *src)
>  static int damos_commit_ops_filters(struct damos *dst, struct damos *src)
>  {
>  	struct damos_filter *dst_filter, *next, *src_filter, *new_filter;
> -	int i = 0, j = 0;
> +	int i = 0, j = 0, err;
>  
>  	damos_for_each_ops_filter_safe(dst_filter, next, dst) {
>  		src_filter = damos_nth_ops_filter(i++, src);
> -		if (src_filter)
> -			damos_commit_filter(dst_filter, src_filter);
> -		else
> +		if (src_filter) {
> +			err = damos_commit_filter(dst_filter, src_filter);
> +			if (err)
> +				return err;
> +		} else {
>  			damos_destroy_filter(dst_filter);
> +		}
>  	}
>  
>  	damos_for_each_ops_filter_safe(src_filter, next, src) {
> @@ -1401,7 +1412,11 @@ static int damos_commit_ops_filters(struct damos *dst, struct damos *src)
>  				src_filter->allow);
>  		if (!new_filter)
>  			return -ENOMEM;
> -		damos_commit_filter_arg(new_filter, src_filter);
> +		err = damos_commit_filter_arg(new_filter, src_filter);
> +		if (err) {
> +			damos_destroy_filter(new_filter);
> +			return err;
> +		}

I re-checked this version, and the error return from
damos_commit_filter_arg() is now handled at all call sites,
including the new-filter paths.

Reviewed-by: Kunwu Chan <kunwu.chan@gmail.com>

I'll take another look at the remaining patches.

Thanks,
Kunwu


>  		damos_add_filter(dst, new_filter);
>  	}
>  	return 0;
> -- 
> 2.47.3
> 

Sent using hkml (https://github.com/sjp38/hackermail)