Re: [PATCH 1/2] mm/damon/core: Use only installed probe in damon_merge_two_regions()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: memory leak of ctx on r2 allocation failure in damon_test_merge_two()
--

commit ade902b9c0636f895031124ead4ff579818b8169
Author: Suhaas Joshi <suhaas@s-joshi.in>

mm/damon/core: Use only installed probe in damon_merge_two_regions()

While merging 2 regions, we iterate over the entire probe_hits[] array,
whose size is determined by the DAMON_MAX_PROBES macro. It is possible,
however, that we have fewer probes installed than DAMON_MAX_PROBES. In such
cases, we end up making redundant iterations. Therefore, to remedy this,
iterate over the list of installed probes instead of iterating over the
entire array. For doing this, start accepting a struct damon_ctx in
damon_merge_two_regions(), and update calling functions to pass this
argument.

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index ef146ca2ae8aa..bea17b68f13c6 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -182,13 +182,27 @@ static void damon_test_merge_two(struct kunit *test)
>  {
>  	struct damon_target *t;
>  	struct damon_region *r, *r2, *r3;
> +	struct damon_probe *p;
> +	struct damon_ctx *ctx;
>  	int i;
>  
> +	p = damon_new_probe();
> +	if (!p)
> +		kunit_skip(test, "probe alloc fail");
> +	ctx = damon_new_ctx();
> +	if (!ctx) {
> +		damon_destroy_probe(p);
> +		kunit_skip(test, "context alloc fail");
> +	}
> +	damon_add_probe(ctx, p);
>  	t = damon_new_target();
> -	if (!t)
> +	if (!t) {
> +		damon_destroy_ctx(ctx);
>  		kunit_skip(test, "target alloc fail");
> +	}
>  	r = damon_new_region(0, 100);
>  	if (!r) {
> +		damon_destroy_ctx(ctx);
>  		damon_free_target(t);
>  		kunit_skip(test, "region alloc fail");
>  	}

[Severity: Medium]
Is there a missing call to damon_destroy_ctx(ctx) if the allocation for
the second region (r2) fails?

Looking at damon_test_merge_two(), if r2 fails to allocate:

	r2 = damon_new_region(100, 300);
	if (!r2) {
		damon_free_target(t);
		kunit_skip(test, "second region alloc fail");
	}

Does this code leak the newly allocated ctx and its contained probe on this
error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007154851.45368-2-suhaas@s-joshi.in?part=1