Re: [PATCH 02/10] mm/damon/core: add damon_new_region() debug_sanity check

[Zenghui Yu <zenghui.yu@linux.dev>]

Hi SeongJae,

On 3/6/26 11:29 PM, SeongJae Park wrote:
> damon_new_region() is supposed to be called with only valid address
> range arguments.  Do the check under DAMON_DEBUG_SANITY.
> 
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
>  mm/damon/core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index f1a97e85824ac..0c1353164ec81 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -109,6 +109,17 @@ int damon_select_ops(struct damon_ctx *ctx, enum damon_ops_id id)
>  	return err;
>  }
>  
> +#ifdef CONFIG_DAMON_DEBUG_SANITY
> +static void damon_verify_new_region(unsigned long start, unsigned long end)
> +{
> +	WARN_ONCE(start >= end, "start %lu >= end %lu\n", start, end);
> +}
> +#else
> +static void damon_verify_new_region(unsigned long start, unsigned long end)
> +{
> +}
> +#endif
> +
>  /*
>   * Construct a damon_region struct
>   *
> @@ -118,6 +129,7 @@ struct damon_region *damon_new_region(unsigned long start, unsigned long end)
>  {
>  	struct damon_region *region;
>  
> +	damon_verify_new_region(start, end);
>  	region = kmem_cache_alloc(damon_region_cache, GFP_KERNEL);
>  	if (!region)
>  		return NULL;

This can be triggered with

	echo Y > /sys/module/damon_sample_mtier/parameters/enabled

because both node{0,1}_{start,end}_addr are 0 if people forget to properly
initialize them. This can be avoided by checking the parameters right
before damon_new_region(). But I'm not sure if this is the correct
solution.

diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
index 775838a23d93..4a5d3fb12e1b 100644
--- a/samples/damon/mtier.c
+++ b/samples/damon/mtier.c
@@ -118,6 +118,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
 	} else {
 		addr.start = promote ? node1_start_addr : node0_start_addr;
 		addr.end = promote ? node1_end_addr : node0_end_addr;
+
+		if (addr.start >= addr.end)
+			goto free_out;
 	}
 
 	region = damon_new_region(addr.start, addr.end);

Thanks,
Zenghui