mm/damon/core: fix unconditionally skip last region

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

Pre-existing issues:
- [High] Quota tracking pointer charge_target_from can get permanently stuck if its corresponding target becomes invalid, freezing scheme application for all subsequent targets indefinitely. --

commit 4d9a4144cc83637a927f5110bc1cc97f4064d9e8
Author: Liew Rui Yan <[email protected]>

mm/damon/core: fix unconditionally skip last region

This commit modifies damos_skip_charged_region() to only reset the quota's charge_target_from and charge_addr_from when the last region is reached. It ensures that the last region of a tracked target is only skipped if it was actually applied or cannot be split.

Quoted 5 lines

mm/damon/core.c +0 -0

index ce8c6f99106ed..89d1c77417450 100644

mm/damon/core.c static bool damos_skip_charged_region(struct damon_target *t, · line 2460

Quoted 9 lines

mm/damon/core.c +1 -0

@@ -2452,36 +2452,39 @@ static bool damos_skip_charged_region(struct damon_target *t,
 {
 	struct damos_quota *quota = &s->quota;
 	unsigned long sz_to_skip;
+	bool skip = false;
 
 	/* Skip previously charged regions */
 	if (quota->charge_target_from) {
 		if (t != quota->charge_target_from)
 			return true;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for quota->charge_target_from to remain stuck indefinitely if its corresponding target becomes invalid?

Looking at kdamond_apply_schemes() in mm/damon/core.c, if a target becomes invalid, it is skipped and damos_apply_target() is never called for it:

    damon_for_each_target(t, c) {
        if (c->ops.target_valid && c->ops.target_valid(t) == false)
            continue;
        damos_apply_target(c, t, max_region_sz);
    }

If the previous quota window ended with quota->charge_target_from pointing to this now-invalid target, wouldn't the 'return true' here in damos_skip_charged_region() cause all regions of all subsequent valid targets to be skipped?

Because the invalid target is never processed, t will never equal quota->charge_target_from again.

Does this mean the pointer is never cleared, freezing scheme application for all targets?

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

From: Liew Rui Yan <[email protected]>

Once quota set, the charge_{target,addr}_from unconditionally skips and resets at the last region of the tracked target, so the last region can be skipped even when it has not been processed.

Example:

1. Target has 2 regions: R1 (0-100 bytes) and R2 (100-200 bytes).
2. Quota is configured to process only 100 bytes per window.
3. Window 1: Processes R1 (0-100). Quota is full. charge_{target, addr}_from is saved at (Target, 100).
4. Window 2: The loop reaches R2. Because R2 is damon_last_region(t), the old code unconditionally returns true, skipping R2 entirely and resetting the charge_{target,addr}_from.

Result: R2 is permanently skipped even though it has never been processed.

However, it is important to note that this is a very minor issue. This is because it is triggered only when the previous window saved/kept charge_{target,addr}_from, and in the next window, all regions except the last region were skipped by damos_skip_charged_region().

Fix this by only resets the charge_{target,addr}_from when last region is reached, only skips when it applied or cannot split.