mm/damon: DAMOS quota controller and paddr migration walk fixes

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsigned integer overflow in DAMON's moving sum and access rate tracking for large intervals. --

commit 1d58ed9728ddbaf4579ec265e453a7b3a233285c
Author: Ravi Jonnalagadda <[email protected]>

mm/damon/core: fix nr_accesses_bp underflow in damon_moving_sum

Guard against unsigned integer underflow when nomvsum/len_window exceeds mvsum. When that subtraction wraps, the moving sum returns a near maximum value and corrupts nr_accesses_bp. If subtrahend > mvsum, return new_value.

mm/damon/core.c int damon_set_region_system_rams_default(struct damon_target *t, · line 3451

Quoted 8 lines

mm/damon/core.c +0 -0

index 3a8725e400c6b..9975f3d9ebfe9 100644
@@ -3449,7 +3449,11 @@ int damon_set_region_system_rams_default(struct damon_target *t,
 static unsigned int damon_moving_sum(unsigned int mvsum, unsigned int nomvsum,
 		unsigned int len_window, unsigned int new_value)
 {

This isn't a bug introduced by this patch, but could the nomvsum argument passed here be silently overflowing?

Looking at damon_update_region_access_rate(), damon_moving_sum() is called with r->last_nr_accesses * 10000:

mm/damon/core.c:damon_update_region_access_rate() {
	...
	if (attrs->sample_interval)
		len_window = damon_max_nr_accesses(attrs);
	r->nr_accesses_bp = damon_moving_sum(r->nr_accesses_bp,
			r->last_nr_accesses * 10000, len_window,
			accessed ? 10000 : 0);
	...
}

DAMON allows configuring aggr_interval / sample_interval up to the maximum 32-bit unsigned integer (for example, 10ms sampling and 1.5 hours aggregation).

This means len_window, which bounds last_nr_accesses, can exceed 429,496.

If this happens, won't the multiplication r->last_nr_accesses * 10000 silently overflow the 32-bit unsigned int limit of 4,294,967,295?

If this overflows, the corrupted, wrapped-around value is passed as nomvsum, which would cause subtrahend to be artificially small.

Additionally, would this cause r->nr_accesses_bp (which accumulates mvsum) to also continuously wrap around 32-bit limits?

Could these silent integer overflows corrupt memory access tracking and lead to erroneous memory management decisions, like reclaiming hot memory?

A proper fix might require bounding len_window to 429,496 or using 64-bit integers.

Guard against unsigned integer underflow when nomvsum/len_window exceeds mvsum.

When that subtraction wraps, the moving sum returns a near-ULONG_MAX value and corrupts nr_accesses_bp.

If subtrahend > mvsum, return new_value: this clamps the moving-sum estimate to the current observation rather than wrapping.

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The safety cap limits the effective quota output (esz), but fails to cap the tuner's internal state (quota->esz_bp), leading to severe integral windup.
- [High] Accumulating the total monitored size into an unsigned long overflows on 32-bit architectures, permanently crippling the scheme quota.
- [High] Applying the safety cap can forcefully reduce the effective quota below min_region_sz, resulting in permanent scheme starvation for small targets.
- [Medium] The safety cap is completely bypassed for statically sized quotas due to an early return, causing inconsistent API behavior.

The DAMOS quota goal tuner can compute an effective size (esz) larger than the total monitored memory because it integrates over cumulative deltas without bounding by the actual workload size. Once esz exceeds total monitored memory, the per-tick "remaining quota" arithmetic stops being meaningful: any scheme can apply to the entire monitored space and "remaining" stays positive indefinitely.

Cap esz to the total size of all currently monitored regions as a final bound after all other quota calculations. Add damon_ctx_total_monitored_sz() helper that sums region sizes across all targets.

Hello Ravi,

Quoted 6 lines · On Sat, 16 May 2026 14:03:54 -0700 Ravi Jonnalagadda <[email protected]>…

The DAMOS quota goal tuner can compute an effective size (esz) larger than the total monitored memory because it integrates over cumulative deltas without bounding by the actual workload size. Once esz exceeds total monitored memory, the per-tick "remaining quota" arithmetic stops being meaningful: any scheme can apply to the entire monitored space and "remaining" stays positive indefinitely.

Nice finding!

Quoted 5 lines

Cap esz to the total size of all currently monitored regions as a final bound after all other quota calculations. Add damon_ctx_total_monitored_sz() helper that sums region sizes across all targets.

You could also make an arbitrary cap by setting the static size quota. That is, if there are not only quota goal but also the size quota and/or time quota, and the different types of quotas disagree about the real quota, DAMOS uses smallest quota. You could read damos_set_effective_quota() code and kernel-doc comment of 'struct damos_quota' for more details.

So you could apply the total monitoring region size cap by setting the size quota of the total monitoring region size. Could that work for you?

Adding the total monitoring region size cap makes sense to me, and I think that will make user experience better. But, if the size quota based cap works, that could also be handled on user space in an easier and even a betetr way. If so, I'd prefer the direction, to reduce kernel code complexity. What do you think?

Thanks,
SJ

[...]

Quoted 26 lines · SeongJae Park

Hello Ravi,

Quoted 6 lines · On Sat, 16 May 2026 14:03:54 -0700 Ravi Jonnalagadda <[email protected]>…

The DAMOS quota goal tuner can compute an effective size (esz) larger than the total monitored memory because it integrates over cumulative deltas without bounding by the actual workload size. Once esz exceeds total monitored memory, the per-tick "remaining quota" arithmetic stops being meaningful: any scheme can apply to the entire monitored space and "remaining" stays positive indefinitely.

Nice finding!

Quoted 5 lines

Cap esz to the total size of all currently monitored regions as a final bound after all other quota calculations. Add damon_ctx_total_monitored_sz() helper that sums region sizes across all targets.

You could also make an arbitrary cap by setting the static size quota. That is, if there are not only quota goal but also the size quota and/or time quota, and the different types of quotas disagree about the real quota, DAMOS uses smallest quota. You could read damos_set_effective_quota() code and kernel-doc comment of 'struct damos_quota' for more details.

Quoted 7 lines

So you could apply the total monitoring region size cap by setting the size quota of the total monitoring region size. Could that work for you?

Adding the total monitoring region size cap makes sense to me, and I think that will make user experience better. But, if the size quota based cap works, that could also be handled on user space in an easier and even a betetr way. If so, I'd prefer the direction, to reduce kernel code complexity. What do you think?

Hello SJ,

Agreed. quota->sz combined with the smallest-quota-wins rule in damos_set_effective_quota does express this cap from userspace without kernel changes, and keeping the kernel side clean is the right call.

If the UX argument carries weight later, I'm happy to respin v2 with sashiko fixes addressed.

The CONSIST quota goal tuner initializes esz_bp to 0, producing an effective quota size (esz) of 1 byte on the first tick. damos_quota_is_full() rejects all regions when esz < min_region_sz (default PAGE_SIZE = 4096), so no regions can be tried and no feedback reaches the tuner — a bootstrapping deadlock.

Quoted 5 lines · On Sat, 16 May 2026 14:03:55 -0700 Ravi Jonnalagadda <[email protected]>…

The CONSIST quota goal tuner initializes esz_bp to 0, producing an effective quota size (esz) of 1 byte on the first tick. damos_quota_is_full() rejects all regions when esz < min_region_sz (default PAGE_SIZE = 4096), so no regions can be tried and no feedback reaches the tuner — a bootstrapping deadlock.

That depend on whether the goal is already [over]-achieved. If the goal is achieved, the tuner will think no change is needed, so keep the effectively-zero quota. If the goal is over-achived, the tuner will think the DAMOS scheme should be less aggressive, but it is already effectively-zero quota, so keep having effectively-zero quota.

If the ogal is under-achived, the logic will iteratively increase the internal esz (esz_bp), until it exceeds the min_region_sz, and finally start making some effects.

So, unless the goal is already [over]-achieved, there is no deadlock. If the goal is already [over]-achieved, why we would want to make DAMOS do something?

Am I missing something?

I'd like to discuss this high level thing first, before digging deep into the details.

Thanks,
SJ

[...]

Quoted 2 lines · SeongJae Park

On Sat, 16 May 2026 14:03:55 -0700 Ravi Jonnalagadda <[email protected]> wrote:

Dropping patches 1 and 3.

Patches 2, 4, and 5 are independent of this scaffolding; I'll reply on each thread separately with the relevant context.

Thanks again for the careful review.

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Calling PageBuddy() on a tail page triggers a kernel panic.
- [High] Unsafe address advancement skips valid memory when starting from an unaligned offset within a folio.
- [High] Speculative access of folio flags on an unreferenced folio risks VM_BUG_ON_FOLIO.
- [Medium] The optimization fails to skip buddy blocks larger than pageblock_order efficiently.
- [Medium] Skipping an entire pageblock when pfn_to_online_page() returns NULL misses valid memory. --

commit 66e620b27c62d5fcd72a28187d8b381c33d31172
Author: Ravi Jonnalagadda <[email protected]>
Subject: mm/damon/paddr: skip free pageblocks in migration walk

This patch optimizes DAMON physical memory walks on sparse address spaces by checking if a pageblock is entirely free or offline, skipping the entire block to reduce kdamond tick time.

Quoted 19 lines

mm/damon/paddr.c +4 -3

index c4738cd5e221e..e844c990987b9 100644
@@ -258,13 +258,32 @@ static unsigned long damon_pa_migrate(struct damon_region *r,
 		unsigned long addr_unit, struct damos *s,
 		unsigned long *sz_filter_passed)
 {
-	phys_addr_t addr, applied;
+	phys_addr_t addr, end, applied;
 	LIST_HEAD(folio_list);
 	struct folio *folio = NULL;
+	unsigned long pfn;
 
 	addr = damon_pa_phys_addr(r->ar.start, addr_unit);
-	while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
-		folio = damon_get_folio(PHYS_PFN(addr));
+	end = damon_pa_phys_addr(r->ar.end, addr_unit);
+	while (addr < end) {

mm/damon/paddr.c static unsigned long damon_pa_migrate(struct damon_region *r, · line 278

Quoted 10 lines

mm/damon/paddr.c +10 -0

+		pfn = PHYS_PFN(addr);
+
+		/* Skip pageblocks that are entirely free. */
+		if (IS_ALIGNED(pfn, pageblock_nr_pages)) {
+			struct page *page = pfn_to_online_page(pfn);
+
+			if (!page) {
+				addr += pageblock_nr_pages * PAGE_SIZE;
+				continue;
+			}

If a memory hole is smaller than a pageblock, can skipping the entire pageblock miss valid, online pages in the remainder of the block?

mm/damon/paddr.c static unsigned long damon_pa_migrate(struct damon_region *r, · line 283

Quoted 5 lines

mm/damon/paddr.c +5 -0

+			if (PageBuddy(page) &&
+			    buddy_order_unsafe(page) >= pageblock_order) {
+				addr += pageblock_nr_pages * PAGE_SIZE;
+				continue;
+			}

Does this code risk a kernel panic when calling PageBuddy() on a tail page?

If a pageblock-aligned PFN points to a tail page of a larger compound page (such as a 1GB HugeTLB page), PageBuddy() would trigger a VM_BUG_ON_PGFLAGS due to the PF_NO_TAIL policy.

Also, if a free buddy block is larger than pageblock_order, does the optimization fall back to stepping by PAGE_SIZE?

The first pageblock is skipped, but on the next iteration, pfn points to the start of the second pageblock, which is a tail page of the large buddy block. Because the PG_buddy flag is only set on the head page, PageBuddy(page) will evaluate to false.

mm/damon/paddr.c static unsigned long damon_pa_migrate(struct damon_region *r, · line 286

Quoted 4 lines

mm/damon/paddr.c +3 -0

+		}
+
+		folio = damon_get_folio(pfn);

This isn't a bug introduced by this patch, but does damon_get_folio() speculatively access folio flags before acquiring a reference?

mm/damon/ops-common.c:damon_get_folio() {
    ...
    if (!folio_test_lru(folio) || !folio_try_get(folio))
        return NULL;
    ...
}

If the page is concurrently freed and reallocated as a tail page of a new compound page, calling folio_test_lru() before folio_try_get() risks a VM_BUG_ON_FOLIO.

Additionally, this wasn't introduced by this patch, but can the physical address advancement skip valid memory when starting from an unaligned offset?

If damon_pa_migrate() starts at an unaligned offset within a large folio, pfn points to a tail page, and damon_get_folio() resolves this to the head folio. The loop then unconditionally advances by the full folio size:

    put_folio:
        addr += folio_size(folio);
        folio_put(folio);

Adding the total folio size to an unaligned starting offset would push addr past the actual end of the folio, skipping pages in the subsequent memory region.

Signature · 2 lines

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

damon_pa_migrate() walks every PFN in a region linearly, calling damon_get_folio() for each one. On sparse physical address spaces (e.g., CXL-attached memory), a single DAMON region can span hundreds of gigabytes where most memory is free and sitting in the buddy allocator. Most page lookups are fruitless and dominate kdamond tick time.

Check at pageblock boundaries (2MB on x86_64) whether the block is entirely free. If the first page of a pageblock is a buddy page at pageblock_order or higher, the entire block is free and can be skipped.
Similarly skip pageblocks where pfn_to_online_page() returns NULL.

This reduces the iteration from O(region_sz / PAGE_SIZE) to O(region_sz / pageblock_sz) + O(populated_pages).

buddy_order_unsafe() is used without zone->lock. A transient false positive (block becomes non-free between the PageBuddy and order checks) costs at most one tick of missed candidates on that block; the next tick re-scans. No correctness consequence as DAMON walks are best-effort.

Quoted 6 lines · On Sat, 16 May 2026 14:03:56 -0700 Ravi Jonnalagadda <[email protected]>…

damon_pa_migrate() walks every PFN in a region linearly, calling damon_get_folio() for each one. On sparse physical address spaces (e.g., CXL-attached memory), a single DAMON region can span hundreds of gigabytes where most memory is free and sitting in the buddy allocator. Most page lookups are fruitless and dominate kdamond tick time.

On sparse address spaces, the problem would be large DAMON regions of offlined memory. The large DAMON regions that nearly all freed memory is another problem that doesn't require the sparse address spaces. If I'm not wrong, the above paragraph could better clarified in my opinion.

Quoted 8 lines

Check at pageblock boundaries (2MB on x86_64) whether the block is entirely free. If the first page of a pageblock is a buddy page at pageblock_order or higher, the entire block is free and can be skipped.
Similarly skip pageblocks where pfn_to_online_page() returns NULL.

Quoted 8 lines

This reduces the iteration from O(region_sz / PAGE_SIZE) to O(region_sz / pageblock_sz) + O(populated_pages).

buddy_order_unsafe() is used without zone->lock. A transient false positive (block becomes non-free between the PageBuddy and order checks) costs at most one tick of missed candidates on that block; the next tick re-scans. No correctness consequence as DAMON walks are best-effort.

I was initially thinking this is a good and reasonable optimization approach. But on the second thought I get below questions.

For large offlined memory space problem, couldn't we simply tune DAMON's monitoring regions boundary to ignore the holes?

For large free memory area, is it reasonable to assume such situations? In production, users will try to utilize as much memory of the system as possible. Then, wouldn't there be such problematically large free memory area?

Could you please enlighten me?

I will hold digging deep until this high level questions are answered.

Thanks,
SJ

[...]

Quoted 35 lines · SeongJae Park

Quoted 6 lines · On Sat, 16 May 2026 14:03:56 -0700 Ravi Jonnalagadda <[email protected]>…

damon_pa_migrate() walks every PFN in a region linearly, calling damon_get_folio() for each one. On sparse physical address spaces (e.g., CXL-attached memory), a single DAMON region can span hundreds of gigabytes where most memory is free and sitting in the buddy allocator. Most page lookups are fruitless and dominate kdamond tick time.

On sparse address spaces, the problem would be large DAMON regions of offlined memory. The large DAMON regions that nearly all freed memory is another problem that doesn't require the sparse address spaces. If I'm not wrong, the above paragraph could better clarified in my opinion.

Quoted 8 lines

Check at pageblock boundaries (2MB on x86_64) whether the block is entirely free. If the first page of a pageblock is a buddy page at pageblock_order or higher, the entire block is free and can be skipped.
Similarly skip pageblocks where pfn_to_online_page() returns NULL.

Quoted 8 lines

This reduces the iteration from O(region_sz / PAGE_SIZE) to O(region_sz / pageblock_sz) + O(populated_pages).

buddy_order_unsafe() is used without zone->lock. A transient false positive (block becomes non-free between the PageBuddy and order checks) costs at most one tick of missed candidates on that block; the next tick re-scans. No correctness consequence as DAMON walks are best-effort.

I was initially thinking this is a good and reasonable optimization approach. But on the second thought I get below questions.

Quoted 9 lines

For large offlined memory space problem, couldn't we simply tune DAMON's monitoring regions boundary to ignore the holes?

For large free memory area, is it reasonable to assume such situations? In production, users will try to utilize as much memory of the system as possible. Then, wouldn't there be such problematically large free memory area?

Could you please enlighten me?

Hi SJ,

You're right on the first point. For static offlined memory holes (memory hotplug gaps, partial socket population, etc.) the right answer is configuring the monitoring region boundaries to exclude them upfront, not making the walk skip them at runtime. The changelog is clearer if I narrow the patch to the free-but- online case.

On the free-online case: I agree large free memory areas are not the steady state on a fully-utilized system. The cases I had in mind are more limited:

- A workload using a small part of a much larger range, with the rest left as headroom (e.g. 64 GB used of a 512 GB range).

- Shared tiers where workloads are allocated and freed on their own timelines. Any single piece of free memory doesn't last long, but on a busy system there's typically a meaningful free fraction in the range at any point -- especially on a slower tier, where workloads prefer faster memory first when it's available.

The patch as written is a narrow optimization for those cases:
the pageblock-aligned check is one extra read per pageblock_nr_pages PFNs (about 1 per 512 on x86_64), so it's effectively a no-op when the region is fully populated.

If you don't see those workloads as warranting the change, I'm happy to drop the patch. If the framing is the issue more than the change itself, I can respin a v2 with:

- the changelog narrowed to the free-but-online case (no

    offlined-memory framing);

- any suggestions from you on sashiko's review comments.

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The amortized time budget check can be permanently bypassed due to variable PFN strides and loop continues.
- [High] Early termination causes permanent starvation for the tail end of large DAMON regions.
- [High] The time budget targets the wrong phase, failing to cap the actual latency spike of the migration.
- [High] PFN advancement after folio conversion permanently misaligns the scan.
--

commit 87860cbf6b4be5f161140aa9a1f9b7d0461955ba
Author: Ravi Jonnalagadda <[email protected]>

mm/damon/paddr: add time budget to migration page walk

This patch adds a 100ms wall-clock time budget to damon_pa_migrate() to prevent long migration walks from causing high scheduling latencies. The time check is amortized by checking ktime_get() every 4096 pages.

Quoted 4 lines

mm/damon/paddr.c +0 -0

index e844c990987b9..a2565287bc10f 100644

[ ... ]

mm/damon/paddr.c static unsigned long damon_pa_migrate(struct damon_region *r, · line 299

Quoted 8 lines

mm/damon/paddr.c +4 -0

@@ -283,6 +293,11 @@ static unsigned long damon_pa_migrate(struct damon_region *r,
 			}
 		}
 
+		/* Time budget: keep kdamond responsive on long migration walks. */
+		if (!(pfn & DAMON_PA_MIGRATE_TIME_CHECK_MASK) &&
+		    ktime_after(ktime_get(), deadline))
+			break;

Can this time check be permanently bypassed?

If pfn advances by variable strides, such as when addr += folio_size(folio) adds a multi-page stride that jumps over the 4096-page boundary, the exact match !(pfn & DAMON_PA_MIGRATE_TIME_CHECK_MASK) is missed, skipping the check for the entire next 4096-page cycle.

Additionally, if pfn does land exactly on a multiple of 4096, but the pageblock is skipped via the continue statement just above this block:

        if (!page) {
            addr += pageblock_nr_pages * PAGE_SIZE;
            continue;
        }

the loop advances and leaps over the time check.

If the loop terminates early here, is there a mechanism to save the PFN where the scan aborted?

Since the DAMON core is unaware of this internal early return, the next time the scheme applies to this region, it appears it will restart the walk from the very beginning of the region.

Could this result in a permanent livelock for large regions, where memory beyond the 100ms scan horizon is never reached or migrated?

Quoted 2 lines

mm/damon/paddr.c +0 -0

 		folio = damon_get_folio(pfn);
 		if (damon_pa_invalid_damos_folio(folio, s)) {

[ ... ]

mm/damon/paddr.c

Quoted 5 lines

mm/damon/paddr.c +0 -0

 		if (!folio_isolate_lru(folio))
 			goto put_folio;
 		list_add(&folio->lru, &folio_list);
 put_folio:
 		addr += folio_size(folio);

This wasn't introduced by this patch, but does the address advancement correctly handle tail pages?

Since the region start addr might not be folio-aligned, if it falls within a tail page of a large folio, does adding the total folio_size(folio) overshoot the end of the current folio and skip the head page of the next folio?

mm/damon/paddr.c

Quoted 4 lines

mm/damon/paddr.c +0 -0

 		folio_put(folio);
 	}
 	applied = damon_migrate_pages(&folio_list, s->target_nid);

Does the time budget cap the actual latency spike of the migration?

The 100ms budget limits the fast folio isolation phase. The loop can isolate gigabytes of memory within 100ms. Once the deadline is reached, all collected folios are passed to damon_migrate_pages().

Could migrating this massive batch, which requires memory allocation and page copying, take seconds to complete in a single blocking call and fail to keep the migration responsive?

Signature · 2 lines

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

On populated physical address ranges the pageblock skip optimization alone is insufficient — most pageblocks contain at least one allocated page, so the walk still iterates millions of PFNs.

Quoted 3 lines · On Sat, 16 May 2026 14:03:57 -0700 Ravi Jonnalagadda <[email protected]>…

On populated physical address ranges the pageblock skip optimization alone is insufficient — most pageblocks contain at least one allocated page, so the walk still iterates millions of PFNs.

So my questions to the fourth patch of this series are also applied here, especially about the assumption of systems having most memory free. I will hold digging deep here until the high level discussion is completed.

Thanks,
SJ

[...]

Quoted 11 lines · SeongJae Park

Quoted 3 lines · On Sat, 16 May 2026 14:03:57 -0700 Ravi Jonnalagadda <[email protected]>…

On populated physical address ranges the pageblock skip optimization alone is insufficient — most pageblocks contain at least one allocated page, so the walk still iterates millions of PFNs.

So my questions to the fourth patch of this series are also applied here, especially about the assumption of systems having most memory free. I will hold digging deep here until the high level discussion is completed.

Hello SJ,

Stepping back to look at this with fresh eyes, I think this patch is in the same bucket as patches 1 and 3 (full background on the patch 3 thread): it came out of the same parallel debug effort, where I was seeing long walks during the startup transient on a multi-hundred-GB monitored target -- before kdamond_split_regions() and damon_apply_min_nr_regions() had trimmed the initial regions down -- and was unsure whether those long walks were contributing to the NMI-side responsiveness issues I was chasing.

Once the actual NMI problem was fixed and the per-region work in steady state is bounded by DAMON's region splitting (and by the scheme's quota when one is set), the per-call cost in damon_pa_migrate() is already small enough that the budget isn't doing useful work. cond_resched() after damon_migrate_pages() covers the preemption case.

If a real workload later shows a per-region walk long enough to matter, I'll re-evaluate then with concrete numbers.