Re: [RFC PATCH] mm/damon/core: reset invalid quota->charge_target_from

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The new `damos_reset_invalid_charge_target_from()` function fails to reset dangling `charge_target_from` pointers for PADDR operations, permanently breaking `damon_reclaim` functionality across restarts.
--

commit 6f10f4f2fc1ae76229671b5157f9aa34aed9227b
Author: SJ Park <sj@kernel.org>

    mm/damon/core: reset invalid quota->charge_target_from

This commit fixes an issue where DAMOS suddenly stops working if a target
process that a quota just fully charged in is terminated. It catches this
corner case and resets the invalid quota->charge_target_from pointer.

Link: https://lore.kernel.org/20260830064708.40CA61F000E9@smtp.kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index ce8c6f99106ed..2c58be9edbde4 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3283,6 +3283,22 @@ static void damos_trace_esz(struct damon_ctx *c, struct damos *s,
>  	trace_damos_esz(cidx, sidx, quota->esz);
>  }
>  
> +static void damos_reset_invalid_charge_target_from(struct damos_quota *quota,
> +		struct damon_ctx *c)
> +{
> +	struct damon_target *t;
> +
> +	t = quota->charge_target_from;
> +	if (!t)
> +		return;
> +	if (!c->ops.target_valid)
> +		return;

[Severity: High]
This is a pre-existing issue, but by returning early here, does this new
function fail to clear dangling charge_target_from pointers for PADDR
operations (like damon_reclaim)?

If damon_reclaim is enabled with quotas and hits quota exhaustion,
quota->charge_target_from is set. If damon_reclaim is then disabled,
damon_stop() destroys the targets, leaving charge_target_from as a dangling
pointer.

When damon_reclaim is re-enabled, a new target is created. Because
target_valid is NULL for PADDR operations, this new function returns early.

Could this lead to damos_skip_charged_region() comparing the new target
against the dangling pointer, evaluating to true, and permanently skipping
all memory reclaim actions?

> +	if (c->ops.target_valid(t))
> +		return;
> +	quota->charge_target_from = NULL;
> +	quota->charge_addr_from = 0;
> +}
>
>  static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)

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