DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH v2] mm/damon/core: reset invalid quota->charge_target_from
@ 2026-09-10 14:28 SJ Park
  2026-09-10 14:43 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: SJ Park @ 2026-09-10 14:28 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SJ Park, stable, Enze Li, damon, linux-kernel, linux-mm

DAMOS can suddenly stop working if a target process that the quota is
just fully charged on is terminated.  Fix by catching and processing the
corner case.

When DAMOS quota is fully charged, the target and the region to continue
applying the action in the next round is saved in
damos_quota->charge_{target,addr}_from.  In the next round, DAMOS
iterates targets and regions from the beginning.  It skips applying the
action to the regions until it visits and skips the saved target/region.

Virtual address space targets become invalid if the process is
terminated.  Trying to apply the scheme to invalid target is just a
waste of time.  Hence commit 6e4930e33329 ("mm/damon/core: fix wasteful
CPU calls by skipping non-existent targets") made the logic to skip
invalid targets.  However, it does skip before the charged target/region
skipping/updating.

Let's suppose the user runs DAMOS for multiple virtual address spaces
with a quota.  The quota exceeded in the middle of a virtual address
space.  And the process of the address space is terminated.  Then the
charge_target_from points to the invalid target.  The pointer update
logic is skipped for the invalid target, so the charge_target_from is
never updated.  DAMOS action to every target/region is skipped.  From
the user's perspective, it would look like suddenly DAMOS has stopped
working.

No critical leak or crash can happen.  The user could reinstall the
scheme.  But this makes use of DAMOS under certain setups quite
unreliable.

When the invalid target is found, further check the corner case and
reset the pointer.

This issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260830064708.40CA61F000E9@smtp.kernel.org

Fixes: 6e4930e33329 ("mm/damon/core: fix wasteful CPU calls by skipping non-existent targets")
Cc: <stable@vger.kernel.org> # 7.0.x
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from RFC v2
- RFC v2: https://lore.kernel.org/20260908144246.104162-1-sj@kernel.org
- Drop RFC tag again.
Changes from v1
- v1: https://lore.kernel.org/20260907165529.98874-1-sj@kernel.org
- Move the corner case check to invalid target found case, to handle
  dynamic quota uninstall case.
- Add RFC tag again.
Changes from RFC v1
- RFC v1: https://lore.kernel.org/20260906183654.95634-1-sj@kernel.org
- Drop RFC tag.

 mm/damon/core.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 645cb367019ae..06cf2ab7e97d7 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3393,8 +3393,15 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 	max_region_sz = damon_region_sz_limit(c);
 	mutex_lock(&c->walk_control_lock);
 	damon_for_each_target(t, c) {
-		if (c->ops.target_valid && c->ops.target_valid(t) == false)
+		if (c->ops.target_valid && c->ops.target_valid(t) == false) {
+			damon_for_each_scheme(s, c) {
+				if (s->quota.charge_target_from != t)
+					continue;
+				s->quota.charge_target_from = NULL;
+				s->quota.charge_addr_from = 0;
+			}
 			continue;
+		}
 		damos_apply_target(c, t, max_region_sz);
 	}
 

base-commit: bd699d8a4388cda037f01797294c834fb422ebe6
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] mm/damon/core: reset invalid quota->charge_target_from
  2026-09-10 14:28 [PATCH v2] mm/damon/core: reset invalid quota->charge_target_from SJ Park
@ 2026-09-10 14:43 ` sashiko-bot
  2026-09-10 14:50   ` SJ Park
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-09-10 14:43 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

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

Pre-existing issues:
- [High] DAMOS scheme processing permanently stalls if a valid target has zero regions, reproducing the exact lockup the patch intended to fix.
--

commit be23af6d135629bfa902e51163989385be75b34b
Author: SJ Park <sj@kernel.org>
One-line subject: mm/damon/core: reset invalid quota->charge_target_from

This commit fixes an issue where DAMOS can suddenly stop working if a target
process that the quota is fully charged on is terminated. It handles this
corner case by catching the invalid target and resetting the charge target
pointer.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 645cb367019ae..06cf2ab7e97d7 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3393,8 +3393,15 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
>  	max_region_sz = damon_region_sz_limit(c);
>  	mutex_lock(&c->walk_control_lock);
>  	damon_for_each_target(t, c) {
> -		if (c->ops.target_valid && c->ops.target_valid(t) == false)
> +		if (c->ops.target_valid && c->ops.target_valid(t) == false) {
> +			damon_for_each_scheme(s, c) {
> +				if (s->quota.charge_target_from != t)
> +					continue;
> +				s->quota.charge_target_from = NULL;
> +				s->quota.charge_addr_from = 0;
> +			}
>  			continue;
> +		}
>  		damos_apply_target(c, t, max_region_sz);

[Severity: High]
This is a pre-existing issue, but does this incomplete fix leave DAMOS
vulnerable to permanently stalling when a valid target has zero regions,
or when using operations where target_valid() is not implemented?

If a DAMOS quota is exhausted while applying actions to a target, saving its
pointer to s->quota.charge_target_from, and before the next apply interval
all regions of that target are dynamically removed but the target itself is
kept valid, kdamond_apply_schemes() will process the target here.

Because the target has zero regions, the loop inside damos_apply_target()
is bypassed:

mm/damon/core.c:damos_apply_target() {
	...
	damon_for_each_region(r, t) {
		...
		damon_do_apply_schemes(c, t, r);
		...
	}
}

Consequently, damos_skip_charged_region() is never executed to clear
charge_target_from.

When evaluating all other targets, damos_skip_charged_region() will check
the stale charge_target_from pointer and unconditionally return true,
skipping all actions and permanently stalling scheme processing.

Could we ensure charge_target_from is cleared even if the target has no
regions, or if the target_valid() operation is missing?

>  	}

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] mm/damon/core: reset invalid quota->charge_target_from
  2026-09-10 14:43 ` sashiko-bot
@ 2026-09-10 14:50   ` SJ Park
  0 siblings, 0 replies; 3+ messages in thread
From: SJ Park @ 2026-09-10 14:50 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Thu, 10 Sep 2026 14:43:34 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] DAMOS scheme processing permanently stalls if a valid target has zero regions, reproducing the exact lockup the patch intended to fix.
> --
> 
> commit be23af6d135629bfa902e51163989385be75b34b
> Author: SJ Park <sj@kernel.org>
> One-line subject: mm/damon/core: reset invalid quota->charge_target_from
> 
> This commit fixes an issue where DAMOS can suddenly stop working if a target
> process that the quota is fully charged on is terminated. It handles this
> corner case by catching the invalid target and resetting the charge target
> pointer.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 645cb367019ae..06cf2ab7e97d7 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3393,8 +3393,15 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
> >  	max_region_sz = damon_region_sz_limit(c);
> >  	mutex_lock(&c->walk_control_lock);
> >  	damon_for_each_target(t, c) {
> > -		if (c->ops.target_valid && c->ops.target_valid(t) == false)
> > +		if (c->ops.target_valid && c->ops.target_valid(t) == false) {
> > +			damon_for_each_scheme(s, c) {
> > +				if (s->quota.charge_target_from != t)
> > +					continue;
> > +				s->quota.charge_target_from = NULL;
> > +				s->quota.charge_addr_from = 0;
> > +			}
> >  			continue;
> > +		}
> >  		damos_apply_target(c, t, max_region_sz);
> 
> [Severity: High]
> This is a pre-existing issue, but does this incomplete fix leave DAMOS
> vulnerable to permanently stalling when a valid target has zero regions,
> or when using operations where target_valid() is not implemented?
> 
> If a DAMOS quota is exhausted while applying actions to a target, saving its
> pointer to s->quota.charge_target_from, and before the next apply interval
> all regions of that target are dynamically removed but the target itself is
> kept valid, kdamond_apply_schemes() will process the target here.
> 
> Because the target has zero regions, the loop inside damos_apply_target()
> is bypassed:
> 
> mm/damon/core.c:damos_apply_target() {
> 	...
> 	damon_for_each_region(r, t) {
> 		...
> 		damon_do_apply_schemes(c, t, r);
> 		...
> 	}
> }
> 
> Consequently, damos_skip_charged_region() is never executed to clear
> charge_target_from.
> 
> When evaluating all other targets, damos_skip_charged_region() will check
> the stale charge_target_from pointer and unconditionally return true,
> skipping all actions and permanently stalling scheme processing.
> 
> Could we ensure charge_target_from is cleared even if the target has no
> regions, or if the target_valid() operation is missing?

No, the issue cannot happen.

If the target has zero region, quota couldn't be fully charged on the target.
So charge_target_from couldn't point the empty region target.

For all operation set that could have invalid target, target_valid() operation
is set.

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


Thanks,
SJ

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-10 14:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-10 14:28 [PATCH v2] mm/damon/core: reset invalid quota->charge_target_from SJ Park
2026-09-10 14:43 ` sashiko-bot
2026-09-10 14:50   ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox