mm/damon/core: reset invalid quota->charge_target_from
Subsystems (3)
SJ Park
maintainerRFC PATCH 1/1 mm/damon/core: reset invalid quota->charge_target_from
DAMOS can suddenly stop working if a target process that quota just fully charged in it 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.
Catch the corner case and reset the pointer in damos_adjust_quota(), which is called for adjustment of the quota for every round.
This issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/[email protected]
--- mm/damon/core.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
mm/damon/core.c +18 -0
@@ -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 ; +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) {struct damos_quota *quota = &s->quota;
@@ -3318,6 +3334,8 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)damos_trace_esz (c, s, quota); } +damos_reset_invalid_charge_target_from (quota, c); +if (!c->ops.get_scheme_score)return ;
base-commit: 98a7a286b6060dce5e93b37e28f2e0be80304027
Signature · 1 line
2.47.3
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 8 lines
mm/damon/core.c +0
-0@@ -3283,6 +3283,22 @@ static void damos_trace_esz(struct damon_ctx *c, struct damos *s,trace_damos_esz (cidx, sidx, quota->esz); }
mm/damon/core.c static void damos_trace_esz(struct damon_ctx *c, struct damos *s, · line 3295
Quoted 10 lines
mm/damon/core.c +10
-0+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?
Quoted 7 lines
mm/damon/core.c +5
-0+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)
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
SJ Park
maintainerQuoted 46 lines · On Sun, 06 Sep 2026 18:58:30 +0000 [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The newdamos_reset_invalid_charge_target_from()function fails to reset danglingcharge_target_frompointers for PADDR operations, permanently breakingdamon_reclaimfunctionality across restarts. --commit
6f10f4f2fc1ae76229671b5157f9aa34aed9227b
Author: SJ Park <[email protected]>mm/damon/core: reset invalid quota->charge_target_fromThis 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_frompointer.Quoted 8 lines
mm/damon/core.c +0
-0@@ -3283,6 +3283,22 @@ static void damos_trace_esz(struct damon_ctx *c, struct damos *s,trace_damos_esz (cidx, sidx, quota->esz); }mm/damon/core.c static void damos_trace_esz(struct damon_ctx *c, struct damos *s, · line 3295
Quoted 10 lines
mm/damon/core.c +10
-0+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 danglingcharge_target_frompointers for PADDR operations (likedamon_reclaim)?If
damon_reclaimis enabled with quotas and hits quota exhaustion,quota->charge_target_fromis set. Ifdamon_reclaimis then disabled,damon_stop()destroys the targets, leavingcharge_target_fromas a dangling pointer.
Quoted 6 lines
When
damon_reclaimis re-enabled, a new target is created. Becausetarget_validis 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?
The problem doesn't exist. DAMON_RECLAIM does not reuse the last scheme. Same for all DAMON API callers.
Quoted 11 lines
Quoted 8 lines
mm/damon/core.c +5
-0+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)Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Thanks,
SJ