mm/damon: fix damos quota walk-position tracking

Quoted 49 lines · On Wed, 26 Aug 2026 07:05:08 -0700 SJ Park <[email protected]> wrote:

Quoted 37 lines · On Wed, 26 Aug 2026 18:24:13 +0800 Liew Rui Yan <[email protected]>…

Quoted 6 lines · On Tue, 25 Aug 2026 06:54:57 -0700 SJ Park <[email protected]> wrote:

Quoted 4 lines · On Tue, 25 Aug 2026 20:46:16 +0800 Liew Rui Yan <[email protected]>…

DAMOS uses charge_target_from/charge_addr_from to remember how far a quota-limited walk has progressed. The current implementation has two problems:

Quoted 7 lines

Quoted 3 lines

1. Once set, the cursor 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.

I don't fully understand this. Could you please clarify more? Maybe adding a realistic example scenario would be helpful.

Problem: Unconditional skip of the last region

In the current damos_skip_charged_region(), there is this logic:

    if (r == damon_last_region(t)) {
        quota->charge_target_from = NULL;
        quota->charge_addr_from = 0;
        return true; /* Skip */
    }

Scenario:
1. Target has 2 regions: R1 (0-100 bytes) and R2 (100-200 bytes).

2. Quota is configured to process only 50 bytes per window.

3. Window 1: Processes R1 (0-50). Quota is full. Cursor is saved at (Target, 50).

4. Window 2: Skips R1 (0-50). Processes R1 (50-100). Quota is full. Cursor is saved at (Target, 100), which is exactly the start of R2.

Quoted 6 lines

5. Window 3: The loop reaches R2. Because R2 is damon_last_region(t), the old code unconditionally returns true, skipping R2 entirely and resetting the cursor.

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

Ok, makes sense. The user impact should be not that big, though.

Quoted 1 line

Quoted 7 lines

Quoted 4 lines

To fix this, the patch advances the cursor every time a region is walked, regardless of whether it is applied or filtered out. This allows DAMON to accurately track whether the last region has already been visited, eliminating the need for the unconditional reset.

Sounds like a big change compared to the problem. Why we cannot modify the last region case? Have you also considered other possible simpler approaches?

For example,

'''

mm/damon/core.c +7 -6

@@ -2686,14 +2686,15 @@ static bool damos_skip_charged_region(struct damon_target *t,
        if (quota->charge_target_from) {
                if (t != quota->charge_target_from)
                        return true;
-               if (r == damon_last_region(t)) {
-                       quota->charge_target_from = NULL;
-                       quota->charge_addr_from = 0;
-                       return true;
-               }
                if (quota->charge_addr_from &&
-                               r->ar.end <= quota->charge_addr_from)
+                               r->ar.end <= quota->charge_addr_from) {
+                       if (r->ar.end == quota->charge_addr_from ||
+                                       r == damon_last_region(t)) {
+                               quota->charge_target_from = NULL;
+                               quota->charge_addr_from = 0;
+                       }
                        return true;
+               }
 
                if (quota->charge_addr_from && r->ar.start <
                                quota->charge_addr_from) {

'''

Quoted 7 lines

Quoted 4 lines

This patch ensures that every target is traversed sequentially and deterministically, even when the quota is set very low. I omitted this benefit in the initial problem description. If you think it is okay, I will add it in the next revision.

What's the problem and benefit? I still don't get it. More clarification would be nice.

Hi SJ,

Apologies for the noise. I accidentally started a new thread due to missing --in-reply-to flag in my git send-email command.

Original thread: [email protected]