Skip to contentSkip to conversation

mm/damon/core: trace esz at first setup

RFC PATCH In review SeongJae Park 2 participants 3 messages May 20

Subsystems (3)
lore
Share & tools
lore

SeongJae Park

author maintainer
#0 lore raw

RFC PATCH 1/1 mm/damon/core: trace esz at first setup

DAMON traces effective size quota from the second update, only if a change has been made by the update. Tracing only changed updates was an intentional decision to avoid unnecessary same value tracing. Always skipping the first value is just an unintended mistake.

The mistake makes the tracepoint based investigation incomplete, because the first effective size quota is never traced. It is not a big issue when the 'consist' quota tuner is used, because it keeps changing the quota in the usual setup.

However, when the 'temporal' tuner is used, the quota value is not changed before the goal achievement status is completely changed. For example, if the DAMOS scheme is started with an under-achieved goal, the quota is set to the maximum value, and kept the same value until the goal is achieved. Because DAMON skips the first value, the user cannot know what effective quota the current scheme is using. Only after the goal is achieved, the effective quota is changed to zero, and traced.

Unconditionally trace the initial quota value to fix this problem.

Note that the 'temporal' quota tuner was introduced by commit af738a6a00c1 ("mm/damon/core: introduce DAMOS_QUOTA_GOAL_TUNER_TEMPORAL"), which was added to 7.1-rc1. But even with the 'consist' quota tuner, the tracing is unintentionally incomplete. Hence this commit marks the introduction of the trace event as the broken commit.

Fixes a86d695193bf ("mm/damon: add trace event for effective size quota")
Cc <[email protected]> # 6.17.x
---
 mm/damon/core.c | 2 ++
 1 file changed, 2 insertions(+)
mm/damon/core.c +2 -0
index 68b3b4bbc8fc9..0267faf216b95 100644
@@ -2886,6 +2886,8 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 	if (!quota->total_charged_sz && !quota->charged_from) {
 		quota->charged_from = jiffies;
 		damos_set_effective_quota(c, s);
+		if (trace_damos_esz_enabled())
+			damos_trace_esz(c, s, quota);
 	}
 
 	/* New charge window starts */

base-commit: abe8c076ff4a1283d77c1c5cb7b975723314aec9

Signature · 1 line
2.47.3
#1 lore raw
Thank 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 monotonic accumulators total_charged_ns and total_charged_sz will rapidly overflow on 32-bit architectures, breaking the throughput estimation logic. --

commit 4cb83c3ed801305fa15e94548668201c275a1037
Author: SeongJae Park <[email protected]>

mm/damon/core: trace esz at first setup

This commit fixes an issue where the initial effective size quota is not traced, which leaves tracepoint investigations incomplete when using the temporal tuner. It updates the quota adjustment logic to unconditionally trace the initial quota value.

mm/damon/core.c static void damos_adjust_quota(struct damon_ctx *c, struct damos *s) · line 2883

Quoted 6 lines
mm/damon/core.c +0 -0
index 4e223857a0f99..0db6530825d1d 100644
@@ -2883,6 +2883,8 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 	if (!quota->total_charged_sz && !quota->charged_from) {

[Severity: High]
This isn't a bug introduced by this patch, but will total_charged_ns and total_charged_sz overflow on 32-bit architectures?

Since unsigned long is 32 bits on those systems, total_charged_ns will overflow after roughly 4.29 seconds of total active execution time.

Additionally, the throughput calculation in damos_set_effective_quota():

damos_set_effective_quota() {
    ...
    if (quota->total_charged_ns)
        throughput = mult_frac(quota->total_charged_sz,
                1000000, quota->total_charged_ns);
    ...
}

uses mult_frac() which evaluates (x % d) * n internally using the type of x. Since the remainder could be up to total_charged_ns - 1, multiplying it by 1,000,000 will quickly overflow the 32-bit boundary. This produces incorrect throughput estimates and impacts the effective size quota calculations on 32-bit systems.

Quoted 7 lines
mm/damon/core.c +2 -0
 		quota->charged_from = jiffies;
 		damos_set_effective_quota(c, s);
+		if (trace_damos_esz_enabled())
+			damos_trace_esz(c, s, quota);
 	}
 
 	/* New charge window starts */
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#2 lore raw
Quoted 4 lines · On Wed, 20 May 2026 01:18:37 +0000 [email protected] wrote:

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

Pre-existing issues:
- [High] The monotonic accumulators total_charged_ns and total_charged_sz will rapidly overflow on 32-bit architectures, breaking the throughput estimation logic.

Not introduced by this patch, so no blocker.

Quoted 36 lines

--

commit 4cb83c3ed801305fa15e94548668201c275a1037
Author: SeongJae Park <[email protected]>

mm/damon/core: trace esz at first setup

This commit fixes an issue where the initial effective size quota is not traced, which leaves tracepoint investigations incomplete when using the temporal tuner. It updates the quota adjustment logic to unconditionally trace the initial quota value.

mm/damon/core.c static void damos_adjust_quota(struct damon_ctx *c, struct damos *s) · line 2883

Quoted 6 lines
mm/damon/core.c +0 -0
index 4e223857a0f99..0db6530825d1d 100644
@@ -2883,6 +2883,8 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 	if (!quota->total_charged_sz && !quota->charged_from) {

[Severity: High]
This isn't a bug introduced by this patch, but will total_charged_ns and total_charged_sz overflow on 32-bit architectures?

Since unsigned long is 32 bits on those systems, total_charged_ns will overflow after roughly 4.29 seconds of total active execution time.

Additionally, the throughput calculation in damos_set_effective_quota():

damos_set_effective_quota() {
    ...
    if (quota->total_charged_ns)
        throughput = mult_frac(quota->total_charged_sz,
                1000000, quota->total_charged_ns);
    ...
}
Quoted 5 lines

uses mult_frac() which evaluates (x % d) * n internally using the type of x. Since the remainder could be up to total_charged_ns - 1, multiplying it by 1,000,000 will quickly overflow the 32-bit boundary. This produces incorrect throughput estimates and impacts the effective size quota calculations on 32-bit systems.

On 32-bit environment, addr_unit can be used to workaround this issue. On systems that runs DAMON after a reasonable amount of tests, the issue should not happen. I'm thinking about a better solution, though.

Anyway, not a blocker of this patch.

Thanks,
SJ

[...]