FAILED: patch "[PATCH] mm/damon/core: fix unconditionally skip last region" failed to apply to 6.6-stable tree

PATCH In review [email protected] 4 participants 21 messages Sep 29 – Oct 11

Subsystems (3)

Stable branch Memory management DAMON

lore

Share & tools

lore

[email protected]

author maintainer

2026-09-29 08:28 UTC #0 lore raw

The patch below does not apply to the 6.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x b3723b596b548c837a766aae3553c14a7b15af2b
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026092948-moonrise-persecute-3597@gregkh' --subject-prefix 'PATCH 6.6.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From b3723b596b548c837a766aae3553c14a7b15af2b Mon Sep 17 00:00:00 2001
From: Liew Rui Yan <[email protected]>
Date: Tue, 8 Sep 2026 06:47:38 -0700
Subject: [PATCH] mm/damon/core: fix unconditionally skip last region

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 resetting the charge_{target,addr}_from when last region is reached, only skipping when it is applied or cannot split.

Link https://lore.kernel.org/[email protected] open here

Fixes 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")

Signed-off-by Liew Rui Yan <[email protected]>

Reviewed-by SJ Park <[email protected]>

Signed-off-by SJ Park <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Cc <[email protected]> # v5.16.x

mm/damon/core.c +14 ~~-11~~

index 2b294fb46648..06a253df8d4b 100644
@@ -2342,36 +2342,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;
-		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)
-			return true;
+				r->ar.end <= quota->charge_addr_from) {
+			skip = true;
+			goto out;
+		}
 
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, min_region_sz);
 			if (!sz_to_skip) {
-				if (damon_sz_region(r) <= min_region_sz)
-					return true;
+				if (damon_sz_region(r) <= min_region_sz) {
+					skip = true;
+					goto out;
+				}
 				sz_to_skip = min_region_sz;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
-			return true;
+			skip = true;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }
 
 static void damos_update_stat(struct damos *s,

SJ Park

maintainer

2026-09-30 09:37 UTC #1 lore raw

PATCH 1/1 mm/damon/core: fix unconditionally skip last region

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:

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

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

Link https://lore.kernel.org/[email protected] open here

Fixes 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")

Signed-off-by Liew Rui Yan <[email protected]>

Reviewed-by SJ Park <[email protected]>

Signed-off-by SJ Park <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Cc <[email protected]> # v5.16.x

(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)

Signed-off-by SJ Park <[email protected]>

---
 mm/damon/core.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

mm/damon/core.c +10 -8

index 0713fda22682c..5ede1733369f5 100644
@@ -883,19 +883,17 @@ static bool damos_skip_charged_region(struct damon_target *t,
 	struct damon_region *r = *rp;
 	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;
-		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)
-			return true;
+				r->ar.end <= quota->charge_addr_from) {
+			skip = true;
+			goto out;
+		}
 
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {

@@ -909,11 +907,15 @@ static bool damos_skip_charged_region(struct damon_target *t,
 			damon_split_region_at(t, r, sz_to_skip);
 			r = damon_next_region(r);
 			*rp = r;
+			skip = false;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }
 
 static void damos_update_stat(struct damos *s,

Signature · 1 line

2.47.3

[email protected]

AI review

2026-09-30 09:54 UTC #2 lore raw

Re: [PATCH 6.6.y] mm/damon/core: fix unconditionally skip last region

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] mm: invalid list head dereference if damon_split_region_at() fails - [Medium] mm: scheme state…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: invalid list head dereference if damon_split_region_at() fails
- [Medium] mm: scheme state leak due to early return for small last region --

commit ae1e4dc8429f6a152575d9c64a1876f9dfb3b7ad
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 charge state when the last region is reached. It ensures the last region is skipped only when it is applied or cannot be split.

Link https://lore.kernel.org/[email protected] open here

Quoted 4 lines

mm/damon/core.c +0 -0

index 0713fda22682c..5ede1733369f5 100644

[ ... ]

mm/damon/core.c

Quoted 7 lines

mm/damon/core.c +0 -0

 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, DAMON_MIN_REGION);
 			if (!sz_to_skip) {
 				if (damon_sz_region(r) <= DAMON_MIN_REGION)
 					return true;

[Severity: Medium]
Does this early return for a small last region bypass the state clearing at the relocated out label?

If damos_skip_charged_region() processes the last region in a target and it is smaller than DAMON_MIN_REGION, the code executes return true early. This bypasses the out block that clears quota->charge_target_from.

In the subsequent window, the scheme could again skip all targets until the saved target, evaluate its last region, and hit the same early return, potentially skipping targets endlessly across windows.

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

Quoted 5 lines

mm/damon/core.c +0 -0

 				sz_to_skip = DAMON_MIN_REGION;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
 			r = damon_next_region(r);
 			*rp = r;

[Severity: High]
Can unconditionally advancing the region pointer after a potentially failed split on the last element cause list head type confusion?

When damos_skip_charged_region() processes the last region of a target and attempts to split it, it calls damon_split_region_at(). If the split fails due to a memory allocation failure, the region is not split and remains the last element in the list.

The subsequent call:
r = damon_next_region(r);

will dereference r->list.next, which points to the list head (&t->regions_list), returning a type-confused pointer.

Passing this bogus pointer back via *rp = r means that subsequent calls to damos_valid_target() and damos_apply_scheme() will dereference it, which could lead to a heap out-of-bounds access.

Quoted 8 lines

mm/damon/core.c +4 -0

+			skip = false;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}

Signature · 1 line

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

SJ Park

maintainer

2026-09-30 10:06 UTC #3 lore raw

Re: [PATCH 6.6.y] mm/damon/core: fix unconditionally skip last region

Quoted 35 lines · On Wed, 30 Sep 2026 09:54:41 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: invalid list head dereference if damon_split_region_at() fails
- [Medium] mm: scheme state leak due to early return for small last region --
commit ae1e4dc8429f6a152575d9c64a1876f9dfb3b7ad
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 charge state when the last region is reached. It ensures the last region is skipped only when it is applied or cannot be split.
Link https://lore.kernel.org/[email protected] open here
Quoted 4 lines
mm/damon/core.c +0 -0
index 0713fda22682c..5ede1733369f5 100644
[ ... ]
mm/damon/core.c
Quoted 7 lines
mm/damon/core.c +0 -0
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, DAMON_MIN_REGION);
 			if (!sz_to_skip) {
 				if (damon_sz_region(r) <= DAMON_MIN_REGION)
 					return true;
[Severity: Medium]
Does this early return for a small last region bypass the state clearing at the relocated out label?

Quoted 7 lines

If damos_skip_charged_region() processes the last region in a target and it is smaller than DAMON_MIN_REGION, the code executes return true early. This bypasses the out block that clears quota->charge_target_from.
In the subsequent window, the scheme could again skip all targets until the saved target, evaluate its last region, and hit the same early return, potentially skipping targets endlessly across windows.

Good catch, I forgot converting this while backporting. I will send v2.

Quoted 19 lines

mm/damon/core.c static bool damos_skip_charged_region(struct damon_target *t, · line 909
Quoted 6 lines
mm/damon/core.c +0 -0
 				sz_to_skip = DAMON_MIN_REGION;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
 			r = damon_next_region(r);
 			*rp = r;
[Severity: High]
Can unconditionally advancing the region pointer after a potentially failed split on the last element cause list head type confusion?
When damos_skip_charged_region() processes the last region of a target and attempts to split it, it calls damon_split_region_at(). If the split fails due to a memory allocation failure, the region is not split and remains the last element in the list.
The subsequent call:
r = damon_next_region(r);

Quoted 6 lines

will dereference r->list.next, which points to the list head (&t->regions_list), returning a type-confused pointer.
Passing this bogus pointer back via *rp = r means that subsequent calls to damos_valid_target() and damos_apply_scheme() will dereference it, which could lead to a heap out-of-bounds access.

Good catch, but pre-existing issue. Maybe a fix that made upstream should be applied here. I will separately work on this.

Quoted 12 lines

Quoted 9 lines

mm/damon/core.c +4 -0

+			skip = false;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}

Signature · 1 line

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

Thanks,
SJ

SJ Park

maintainer

2026-09-30 10:08 UTC #4 lore raw

Re: [PATCH 6.6.y] mm/damon/core: fix unconditionally skip last region

In reply to SJ Park

Hello stable team,

Please ignore this patch. I made a mistake while backporting. I will send v2. Sashiko found the mistake. Please refer to my reply [1] to Sashiko for more details.

[1] https://lore.kernel.org/[email protected]

Thanks,
SJ

[...]

SJ Park

maintainer

2026-09-30 10:18 UTC #5 lore raw

PATCH v2 1/1 mm/damon/core: fix unconditionally skip last region

In reply to [email protected]

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:

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

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

Link https://lore.kernel.org/[email protected] open here

Fixes 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")

Signed-off-by Liew Rui Yan <[email protected]>

Reviewed-by SJ Park <[email protected]>

Signed-off-by SJ Park <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Cc <[email protected]> # v5.16.x

(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)

Signed-off-by SJ Park <[email protected]>

---
Changes from v1
- v1: https://lore.kernel.org/[email protected]
- Add missed damon_sz_region(r) <= min_region_sz case change.

 mm/damon/core.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

mm/damon/core.c +14 ~~-10~~

index 0713fda22682c..c31683c12dd48 100644
@@ -883,37 +883,41 @@ static bool damos_skip_charged_region(struct damon_target *t,
 	struct damon_region *r = *rp;
 	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;
-		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)
-			return true;
+				r->ar.end <= quota->charge_addr_from) {
+			skip = true;
+			goto out;
+		}
 
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, DAMON_MIN_REGION);
 			if (!sz_to_skip) {
-				if (damon_sz_region(r) <= DAMON_MIN_REGION)
-					return true;
+				if (damon_sz_region(r) <= DAMON_MIN_REGION) {
+					skip = true;
+					goto out;
+				}
 				sz_to_skip = DAMON_MIN_REGION;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
 			r = damon_next_region(r);
 			*rp = r;
+			skip = false;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }
 
 static void damos_update_stat(struct damos *s,

Signature · 1 line

2.47.3

[email protected]

AI review

2026-09-30 10:31 UTC #6 lore raw

Re: [PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] mm/damon: list head type confusion and memory corruption on allocation failure --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: list head type confusion and memory corruption on allocation failure --

commit 47762d8f5b1f74489d2b1ca149de65b77ac867a2
Author: Liew Rui Yan <[email protected]>

mm/damon/core: fix unconditionally skip last region

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. Fix this by only resetting the charge_{target,addr}_from when last region is reached, only skipping when it is applied or cannot split.

Link https://lore.kernel.org/[email protected] open here

Quoted 14 lines

mm/damon/core.c +1 -0

index 0713fda22682c..c31683c12dd48 100644
@@ -883,37 +883,41 @@ static bool damos_skip_charged_region(struct damon_target *t,
 	struct damon_region *r = *rp;
 	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;

[ ... ]

Quoted 7 lines

mm/damon/core.c +0 -2

 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, DAMON_MIN_REGION);
 			if (!sz_to_skip) {
-				if (damon_sz_region(r) <= DAMON_MIN_REGION)
-					return true;

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

Quoted 10 lines

mm/damon/core.c +5 -0

+				if (damon_sz_region(r) <= DAMON_MIN_REGION) {
+					skip = true;
+					goto out;
+				}
 				sz_to_skip = DAMON_MIN_REGION;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
 			r = damon_next_region(r);
 			*rp = r;
+			skip = false;

[Severity: High]
If damon_split_region_at() fails to allocate memory for the new region, it silently returns without splitting. When r is the last region, damon_next_region() wraps around to the list head, resulting in an invalid struct damon_region pointer. Can this cause an invalid pointer to be assigned to *rp?

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

Quoted 10 lines

mm/damon/core.c +4 -1

 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }

[Severity: High]
If the allocation failed and r evaluates to a bogus list head pointer, this check for damon_last_region() will fail. The function will then return skip = false, causing the caller in damon_do_apply_schemes() to proceed with the invalid pointer.

Could this lead to out-of-bounds reads in damos_valid_target() and out-of-bounds writes in damos_apply_scheme() when the pointer is dereferenced?

Signature · 1 line

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

SJ Park

maintainer

2026-09-30 10:35 UTC #7 lore raw

Re: [PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

Quoted 76 lines · On Wed, 30 Sep 2026 10:31:50 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: list head type confusion and memory corruption on allocation failure --
commit 47762d8f5b1f74489d2b1ca149de65b77ac867a2
Author: Liew Rui Yan <[email protected]>
mm/damon/core: fix unconditionally skip last region
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. Fix this by only resetting the charge_{target,addr}_from when last region is reached, only skipping when it is applied or cannot split.
Link https://lore.kernel.org/[email protected] open here
Quoted 14 lines
mm/damon/core.c +1 -0
index 0713fda22682c..c31683c12dd48 100644
@@ -883,37 +883,41 @@ static bool damos_skip_charged_region(struct damon_target *t,
 	struct damon_region *r = *rp;
 	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;
[ ... ]
Quoted 7 lines
mm/damon/core.c +0 -2
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, DAMON_MIN_REGION);
 			if (!sz_to_skip) {
-				if (damon_sz_region(r) <= DAMON_MIN_REGION)
-					return true;
mm/damon/core.c static bool damos_skip_charged_region(struct damon_target *t, · line 912
Quoted 10 lines
mm/damon/core.c +5 -0
+				if (damon_sz_region(r) <= DAMON_MIN_REGION) {
+					skip = true;
+					goto out;
+				}
 				sz_to_skip = DAMON_MIN_REGION;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
 			r = damon_next_region(r);
 			*rp = r;
+			skip = false;
[Severity: High]
If damon_split_region_at() fails to allocate memory for the new region, it silently returns without splitting. When r is the last region, damon_next_region() wraps around to the list head, resulting in an invalid struct damon_region pointer. Can this cause an invalid pointer to be assigned to *rp?
mm/damon/core.c static bool damos_skip_charged_region(struct damon_target *t, · line 921
Quoted 10 lines
mm/damon/core.c +4 -1
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }
[Severity: High]
If the allocation failed and r evaluates to a bogus list head pointer, this check for damon_last_region() will fail. The function will then return skip = false, causing the caller in damon_do_apply_schemes() to proceed with the invalid pointer.

Quoted 3 lines

Could this lead to out-of-bounds reads in damos_valid_target() and out-of-bounds writes in damos_apply_scheme() when the pointer is dereferenced?

I will separately work on this. Refer to my other reply to same issue report for details.

[1] https://lore.kernel.org/[email protected]

Quoted 3 lines

Signature · 2 lines

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

Thanks,
SJ

Sasha Levin

maintainer

2026-10-02 14:20 UTC #8 lore raw

Re: [PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

In reply to SJ Park

Quoted 2 lines

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

I'll hold this until the damon_split_region_at() failure handling fix you mentioned is posted, and then take 6.12, 6.6 and 6.1 together with it.

Signature · 2 lines

Thanks,
Sasha

SJ Park

maintainer

2026-10-02 18:04 UTC #9 lore raw

Re: [PATCH 6.6.y v2] mm/damon/core: fix unconditionally skip last region

Quoted 6 lines · On Fri, 2 Oct 2026 10:19:50 -0400 Sasha Levin <[email protected]> wrote:

Quoted 2 lines
Fix this by only resetting the charge_{target,addr}_from when last region is reached, only skipping when it is applied or cannot split.
I'll hold this until the damon_split_region_at() failure handling fix you mentioned is posted, and then take 6.12, 6.6 and 6.1 together with it.

Makes perfect sense to me. I will post the fix with this patch.

Thanks,
SJ

[...]

Sasha Levin

maintainer

2026-09-30 11:14 UTC #10 lore raw

PATCH 1/3 mm/damon/core: fix wasteful CPU calls by skipping non-existent targets

In reply to [email protected]

From: Enze Li <[email protected]>

[ Upstream commit 6e4930e33329eec80dd245f28b52202271f5fb28 ]

Currently, DAMON does not proactively clean up invalid monitoring targets during its runtime. When some monitored processes exit, DAMON continues to make the following unnecessary function calls,

--damon_for_each_target--
--damon_for_each_region--
damon_do_apply_schemes
damos_apply_scheme
damon_va_apply_scheme
damos_madvise
damon_get_mm

it is only in the damon_get_mm() function that it may finally discover the target no longer exists, which wastes CPU resources. A simple idea is to check for the existence of monitoring targets within the kdamond_need_stop() function and promptly clean up non-existent targets.

However, SJ pointed out that this approach is problematic because the online commit logic incorrectly uses list indices to update the monitoring state. This can lead to data loss if the target list is changed concurrently. Meanwhile, SJ suggests checking for target existence at the damon_for_each_target level, and if a target does not exist, simply skip it and proceed to the next one.

Link https://lkml.kernel.org/r/[email protected] open here

Signed-off-by Enze Li <[email protected]>

Reviewed-by SeongJae Park <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Stable-dep-of: b3723b596b54 ("mm/damon/core: fix unconditionally skip last region")

Signed-off-by Sasha Levin <[email protected]>

---
 mm/damon/core.c | 3 +++
 1 file changed, 3 insertions(+)

mm/damon/core.c +3 -0

index 0713fda22682c..2990367d849eb 100644
@@ -1156,6 +1156,9 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 		return;
 
 	damon_for_each_target(t, c) {
+		if (c->ops.target_valid && c->ops.target_valid(t) == false)
+			continue;
+
 		damon_for_each_region_safe(r, next_r, t)
 			damon_do_apply_schemes(c, t, r);
 	}

Signature · 1 line

2.53.0

Sasha Levin

maintainer

2026-09-30 11:14 UTC #11 lore raw

PATCH 2/3 mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

From: SeongJae Park <[email protected]>

[ Upstream commit 1745ccbd2907db2bdaa843e4abccde4fdaccbe5d ]

kdamond_apply_schemes() is using damon_for_each_region_safe(), which is safe for deallocation of the region inside the loop. However, the loop internal logic does not deallocate regions. Hence it is only wasting the next pointer. Also, it causes a problem.

When an address filter is applied, and there is a region that intersects with the filter, the filter splits the region on the filter boundary. The intention is to let DAMOS apply action to only filtered-in address ranges. However, it is using damon_for_each_region_safe(), which sets the next region before the execution of the iteration. Hence, the region that split and now will be next to the previous region, is simply ignored. As a result, DAMOS applies the action to target regions bit slower than expected, when the address filter is used. Shouldn't be a big problem but definitely better to be fixed. damos_skip_charged_region() was working around the issue using a double pointer hack.

Use damon_for_each_region(), which is safe for this use case. And drop the work around in damos_skip_charged_region().

Link https://lkml.kernel.org/r/[email protected] open here

Signed-off-by SeongJae Park <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

[Stable backport: retain the min_region_sz argument of the existing quota-skip helper and pass DAMON_MIN_REGION, since this tree has no per-context minimum region size. Use that argument for the helper's alignment and split checks, preserving the stable minimum (including the KUnit override) while matching the context required by b3723b596b54. Update the helper's parameter documentation for the single region pointer. No functions or context fields are added.]

Stable-dep-of: b3723b596b54 ("mm/damon/core: fix unconditionally skip last region")

Signed-off-by Sasha Levin <[email protected]>

---
 mm/damon/core.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

mm/damon/core.c +17 ~~-15~~

index 2990367d849eb..d2ebfb6e34e3d 100644
@@ -859,8 +859,9 @@ static bool damos_valid_target(struct damon_ctx *c, struct damon_target *t,
  * damos_skip_charged_region() - Check if the given region or starting part of
  * it is already charged for the DAMOS quota.
  * @t:	The target of the region.
- * @rp:	The pointer to the region.
+ * @r:	The region.
  * @s:	The scheme to be applied.
+ * @min_region_sz:	Minimum region size.
  *
  * If a quota of a scheme has exceeded in a quota charge window, the scheme's
  * action would applied to only a part of the target access pattern fulfilling

@@ -871,16 +872,18 @@ static bool damos_valid_target(struct damon_ctx *c, struct damon_target *t,
  * This function checks if a given region should be skipped or not for the
  * reason.  If only the starting part of the region has previously charged,
  * this function splits the region into two so that the second one covers the
- * area that not charged in the previous charge widnow and saves the second
- * region in *rp and returns false, so that the caller can apply DAMON action
- * to the second one.
+ * area that not charged in the previous charge widnow, and return true.  The
+ * caller can see the second one on the next iteration of the region walk.
+ * Note that this means the caller should use damon_for_each_region() instead
+ * of damon_for_each_region_safe().  If damon_for_each_region_safe() is used,
+ * the second region will just be ignored.
  *
- * Return: true if the region should be entirely skipped, false otherwise.
+ * Return: true if the region should be skipped, false otherwise.
  */
 static bool damos_skip_charged_region(struct damon_target *t,
-		struct damon_region **rp, struct damos *s)
+		struct damon_region *r, struct damos *s,
+		unsigned long min_region_sz)
 {
-	struct damon_region *r = *rp;
 	struct damos_quota *quota = &s->quota;
 	unsigned long sz_to_skip;

@@ -900,15 +903,14 @@ static bool damos_skip_charged_region(struct damon_target *t,
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
-					r->ar.start, DAMON_MIN_REGION);
+					r->ar.start, min_region_sz);
 			if (!sz_to_skip) {
-				if (damon_sz_region(r) <= DAMON_MIN_REGION)
+				if (damon_sz_region(r) <= min_region_sz)
 					return true;
-				sz_to_skip = DAMON_MIN_REGION;
+				sz_to_skip = min_region_sz;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
-			r = damon_next_region(r);
-			*rp = r;
+			return true;
 		}
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;

@@ -1045,7 +1047,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (quota->esz && quota->charged_sz >= quota->esz)
 			continue;
 
-		if (damos_skip_charged_region(t, &r, s))
+		if (damos_skip_charged_region(t, r, s, DAMON_MIN_REGION))
 			continue;
 
 		if (!damos_valid_target(c, t, r, s))

@@ -1134,7 +1136,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 static void kdamond_apply_schemes(struct damon_ctx *c)
 {
 	struct damon_target *t;
-	struct damon_region *r, *next_r;
+	struct damon_region *r;
 	struct damos *s;
 	unsigned long sample_interval = c->attrs.sample_interval ?
 		c->attrs.sample_interval : 1;

@@ -1159,7 +1161,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 		if (c->ops.target_valid && c->ops.target_valid(t) == false)
 			continue;
 
-		damon_for_each_region_safe(r, next_r, t)
+		damon_for_each_region(r, t)
 			damon_do_apply_schemes(c, t, r);
 	}

Signature · 1 line

2.53.0

Sasha Levin

maintainer

2026-09-30 11:14 UTC #12 lore raw

PATCH 3/3 mm/damon/core: fix unconditionally skip last region

In reply to Sasha Levin

From: Liew Rui Yan <[email protected]>

[ Upstream commit b3723b596b548c837a766aae3553c14a7b15af2b ]

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:

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

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

Link https://lore.kernel.org/[email protected] open here

Fixes 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")

Signed-off-by Liew Rui Yan <[email protected]>

Reviewed-by SJ Park <[email protected]>

Signed-off-by SJ Park <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Cc <[email protected]> # v5.16.x

Signed-off-by Sasha Levin <[email protected]>

---
 mm/damon/core.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

mm/damon/core.c +14 ~~-11~~

index d2ebfb6e34e3d..82d806d0e2124 100644
@@ -886,36 +886,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;
-		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)
-			return true;
+				r->ar.end <= quota->charge_addr_from) {
+			skip = true;
+			goto out;
+		}
 
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, min_region_sz);
 			if (!sz_to_skip) {
-				if (damon_sz_region(r) <= min_region_sz)
-					return true;
+				if (damon_sz_region(r) <= min_region_sz) {
+					skip = true;
+					goto out;
+				}
 				sz_to_skip = min_region_sz;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
-			return true;
+			skip = true;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }
 
 static void damos_update_stat(struct damos *s,

Signature · 1 line

2.53.0

SJ Park

maintainer

2026-10-09 13:50 UTC #13 lore raw

v3 cover mm/damon/core: fix last region handling of damos_skip_charged_regions()

In reply to [email protected]

Patch 1 is a dependency of patch 2. Without it, patch 2 introduces out-of-bounds memory access bug that was found by Sashiko. Patch 2 fixes a bug that categorized to be backported to stable@.

Changes from v2
- v2: https://lore.kernel.org/[email protected]
- Fix out-of-bounds memory access bug by adding patch 1. Changes from v1
- v1: https://lore.kernel.org/[email protected]
- Add missed damon_sz_region(r) <= min_region_sz case change.

Liew Rui Yan (1):
mm/damon/core: fix unconditionally skip last region

SeongJae Park (1):
mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

 mm/damon/core.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

base-commit: ae7bc7c9b4336d7a8fb4bcfe0a6d8c925bde3ce9

Signature · 1 line

2.47.3

SJ Park

maintainer

2026-10-09 13:50 UTC #14 lore raw

PATCH v3 1/2 mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

From: SeongJae Park <[email protected]>

Use damon_for_each_region(), which is safe for this use case. And drop the work around in damos_skip_charged_region().

Link https://lkml.kernel.org/r/[email protected] open here

Signed-off-by SeongJae Park <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

(cherry picked from commit 1745ccbd2907db2bdaa843e4abccde4fdaccbe5d)

Signed-off-by SJ Park <[email protected]>

---
 mm/damon/core.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

mm/damon/core.c +11 ~~-11~~

index fe91b296f4c6..c86cac855b9b 100644
@@ -871,16 +871,17 @@ static bool damos_valid_target(struct damon_ctx *c, struct damon_target *t,
  * This function checks if a given region should be skipped or not for the
  * reason.  If only the starting part of the region has previously charged,
  * this function splits the region into two so that the second one covers the
- * area that not charged in the previous charge widnow and saves the second
- * region in *rp and returns false, so that the caller can apply DAMON action
- * to the second one.
+ * area that not charged in the previous charge widnow, and return true.  The
+ * caller can see the second one on the next iteration of the region walk.
+ * Note that this means the caller should use damon_for_each_region() instead
+ * of damon_for_each_region_safe().  If damon_for_each_region_safe() is used,
+ * the second region will just be ignored.
  *
- * Return: true if the region should be entirely skipped, false otherwise.
+ * Return: true if the region should be skipped, false otherwise.
  */
 static bool damos_skip_charged_region(struct damon_target *t,
-		struct damon_region **rp, struct damos *s)
+		struct damon_region *r, struct damos *s)
 {
-	struct damon_region *r = *rp;
 	struct damos_quota *quota = &s->quota;
 	unsigned long sz_to_skip;

@@ -907,8 +908,7 @@ static bool damos_skip_charged_region(struct damon_target *t,
 				sz_to_skip = DAMON_MIN_REGION;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
-			r = damon_next_region(r);
-			*rp = r;
+			return true;
 		}
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;

@@ -1045,7 +1045,7 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
 		if (quota->esz && quota->charged_sz >= quota->esz)
 			continue;
 
-		if (damos_skip_charged_region(t, &r, s))
+		if (damos_skip_charged_region(t, r, s))
 			continue;
 
 		if (!damos_valid_target(c, t, r, s))

@@ -1134,7 +1134,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 static void kdamond_apply_schemes(struct damon_ctx *c)
 {
 	struct damon_target *t;
-	struct damon_region *r, *next_r;
+	struct damon_region *r;
 	struct damos *s;
 	unsigned long sample_interval = c->attrs.sample_interval ?
 		c->attrs.sample_interval : 1;

@@ -1156,7 +1156,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 		return;
 
 	damon_for_each_target(t, c) {
-		damon_for_each_region_safe(r, next_r, t)
+		damon_for_each_region(r, t)
 			damon_do_apply_schemes(c, t, r);
 	}

Signature · 1 line

2.47.3

[email protected]

AI review

2026-10-09 14:02 UTC #15 lore raw

Re: [PATCH 6.6.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] mm: damon: double processing of split regions by earlier schemes --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: damon: double processing of split regions by earlier schemes --

commit de615e5101d6a41f308fef487cc464f7d25abb6e
Author: SeongJae Park <[email protected]>

mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

This commit replaces the safe region iterator with damon_for_each_region() in kdamond_apply_schemes() to ensure newly split regions are not ignored during iteration. It also simplifies damos_skip_charged_region() by removing a workaround that updated the region pointer directly.

Link https://lkml.kernel.org/r/[email protected] open here

Quoted 4 lines

mm/damon/core.c +0 -0

index fe91b296f4c66..c86cac855b9b0 100644

[ ... ]

Quoted 10 lines

mm/damon/core.c +1 -1

@@ -1134,7 +1134,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 static void kdamond_apply_schemes(struct damon_ctx *c)
 {
 	struct damon_target *t;
-	struct damon_region *r, *next_r;
+	struct damon_region *r;
 	struct damos *s;
 	unsigned long sample_interval = c->attrs.sample_interval ?
 		c->attrs.sample_interval : 1;

mm/damon/core.c static void kdamond_apply_schemes(struct damon_ctx *c) · line 1160

Quoted 6 lines

mm/damon/core.c +1 -1

@@ -1156,7 +1156,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 		return;
 
 	damon_for_each_target(t, c) {
-		damon_for_each_region_safe(r, next_r, t)
+		damon_for_each_region(r, t)
 			damon_do_apply_schemes(c, t, r);

[Severity: Medium]
Will this change cause earlier schemes to re-process newly split regions?

When damon_do_apply_schemes() evaluates a region, it sequentially iterates through all schemes. If an earlier scheme processes the region, it applies its action and updates its quota.

If a later scheme then hits an address filter or quota while evaluating the same region, it splits the region, shrinking it and inserting a new remainder region immediately after it in the target list.

Because the outer loop now dynamically fetches the next region via damon_for_each_region(), this newly inserted remainder region is evaluated in the very next iteration.

When damon_do_apply_schemes() processes this remainder region, it starts over from the first scheme. Won't this cause the earlier scheme to re-evaluate and apply its action a second time, potentially artificially double-charging its quota and redundantly applying physical memory operations?

Quoted 2 lines

mm/damon/core.c +0 -0
 	}
 

Signature · 1 line

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

SJ Park

maintainer

2026-10-09 14:07 UTC #16 lore raw

Re: [PATCH 6.6.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

Quoted 51 lines · On Fri, 09 Oct 2026 14:02:33 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: damon: double processing of split regions by earlier schemes --
commit de615e5101d6a41f308fef487cc464f7d25abb6e
Author: SeongJae Park <[email protected]>
mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
This commit replaces the safe region iterator with damon_for_each_region() in kdamond_apply_schemes() to ensure newly split regions are not ignored during iteration. It also simplifies damos_skip_charged_region() by removing a workaround that updated the region pointer directly.
Link https://lkml.kernel.org/r/[email protected] open here
Quoted 4 lines
mm/damon/core.c +0 -0
index fe91b296f4c66..c86cac855b9b0 100644
[ ... ]
Quoted 10 lines
mm/damon/core.c +1 -1
@@ -1134,7 +1134,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 static void kdamond_apply_schemes(struct damon_ctx *c)
 {
 	struct damon_target *t;
-	struct damon_region *r, *next_r;
+	struct damon_region *r;
 	struct damos *s;
 	unsigned long sample_interval = c->attrs.sample_interval ?
 		c->attrs.sample_interval : 1;
mm/damon/core.c static void kdamond_apply_schemes(struct damon_ctx *c) · line 1160
Quoted 6 lines
mm/damon/core.c +1 -1
@@ -1156,7 +1156,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
 		return;
 
 	damon_for_each_target(t, c) {
-		damon_for_each_region_safe(r, next_r, t)
+		damon_for_each_region(r, t)
 			damon_do_apply_schemes(c, t, r);
[Severity: Medium]
Will this change cause earlier schemes to re-process newly split regions?
When damon_do_apply_schemes() evaluates a region, it sequentially iterates through all schemes. If an earlier scheme processes the region, it applies its action and updates its quota.
If a later scheme then hits an address filter or quota while evaluating the same region, it splits the region, shrinking it and inserting a new remainder region immediately after it in the target list.

Quoted 8 lines

Because the outer loop now dynamically fetches the next region via damon_for_each_region(), this newly inserted remainder region is evaluated in the very next iteration.
When damon_do_apply_schemes() processes this remainder region, it starts over from the first scheme. Won't this cause the earlier scheme to re-evaluate and apply its action a second time, potentially artificially double-charging its quota and redundantly applying physical memory operations?

Minor impact issue that also exist in the mainline. Hence no blocker of this series in my opinion. I will separately work on the issue.

Quoted 7 lines

Quoted 3 lines
mm/damon/core.c +0 -0
 	}
 
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thanks,
SJ

SJ Park

maintainer

2026-10-09 13:50 UTC #17 lore raw

PATCH v3 2/2 mm/damon/core: fix unconditionally skip last region

In reply to SJ Park

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:

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

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

Link https://lore.kernel.org/[email protected] open here

Fixes 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")

Signed-off-by Liew Rui Yan <[email protected]>

Reviewed-by SJ Park <[email protected]>

Signed-off-by SJ Park <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

Cc <[email protected]> # v5.16.x

(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)

Signed-off-by SJ Park <[email protected]>

---
 mm/damon/core.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

mm/damon/core.c +14 ~~-11~~

index c86cac855b9b..c459eae4bbae 100644
@@ -884,36 +884,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;
-		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)
-			return true;
+				r->ar.end <= quota->charge_addr_from) {
+			skip = true;
+			goto out;
+		}
 
 		if (quota->charge_addr_from && r->ar.start <
 				quota->charge_addr_from) {
 			sz_to_skip = ALIGN_DOWN(quota->charge_addr_from -
 					r->ar.start, DAMON_MIN_REGION);
 			if (!sz_to_skip) {
-				if (damon_sz_region(r) <= DAMON_MIN_REGION)
-					return true;
+				if (damon_sz_region(r) <= DAMON_MIN_REGION) {
+					skip = true;
+					goto out;
+				}
 				sz_to_skip = DAMON_MIN_REGION;
 			}
 			damon_split_region_at(t, r, sz_to_skip);
-			return true;
+			skip = true;
 		}
+	}
+out:
+	if (r == damon_last_region(t)) {
 		quota->charge_target_from = NULL;
 		quota->charge_addr_from = 0;
 	}
-	return false;
+	return skip;
 }
 
 static void damos_update_stat(struct damos *s,

Signature · 1 line

2.47.3

[email protected]

AI review

2026-10-09 14:05 UTC #18 lore raw

Re: [PATCH 6.6.y v3 2/2] mm/damon/core: fix unconditionally skip last region

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] mm/damon: dangling charge_target_from stalls schemes on target deletion - [High] mm/damon: missing…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm/damon: dangling charge_target_from stalls schemes on target deletion
- [High] mm/damon: missing charge_target_from reset when target has no regions --

commit 34b3a9a3bd49604e23a1fa56349694c2bc0db52b
Author: Liew Rui Yan <[email protected]>

mm/damon/core: fix unconditionally skip last region

This commit modifies the quota charge skipping logic to only reset charge_target_from and charge_addr_from when the last region is actually reached. It addresses a minor issue where the last region of a tracked target could be unconditionally skipped and reset even when it had not been processed yet.

Link https://lore.kernel.org/[email protected] open here

Quoted 5 lines

mm/damon/core.c +0 -0

index c86cac855b9b0..c459eae4bbaeb 100644

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

Quoted 9 lines

mm/damon/core.c +1 -0

@@ -884,36 +884,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]
Does this logic stall the scheme indefinitely if charge_target_from points to a destroyed target?

If a target is removed during a sysfs commit, damon_sysfs_set_targets() dynamically destroys it:

mm/damon/sysfs.c:damon_sysfs_set_targets() {
[ ... ]

		} else {
			if (damon_target_has_pid(ctx))
				put_pid(t->pid);
			damon_destroy_target(t);
		}

[ ... ]
}

However, damon_sysfs_set_schemes() merely updates existing schemes in-place and fails to clear the quota->charge_target_from pointer if it points to the deleted target.

Because this saved target was deleted and removed from the active target list, would this condition evaluate to true for all remaining targets, permanently skipping all regions and never reaching the logic to clear the dangling pointer?

[Severity: High]
Does this also cause a stall if a target loses all of its regions?

If quota->charge_target_from points to a target that loses all its regions (for example, if all VMAs are unmapped and the task dies), damon_for_each_region() will execute zero times for that target:

mm/damon/core.c:kdamond_apply_schemes() {
[ ... ]

	damon_for_each_target(t, c) {
		damon_for_each_region(r, t)
			damon_do_apply_schemes(c, t, r);
	}

[ ... ]
}

Would this bypass the invocation of damos_skip_charged_region() entirely for the charged target, preventing charge_target_from from being cleared, and causing all subsequent targets to be skipped indefinitely because they do not match the uncleared charge_target_from pointer?

Signature · 1 line

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

SJ Park

maintainer

2026-10-09 14:08 UTC #19 lore raw

Re: [PATCH 6.6.y v3 2/2] mm/damon/core: fix unconditionally skip last region

Quoted 71 lines · On Fri, 09 Oct 2026 14:05:09 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm/damon: dangling charge_target_from stalls schemes on target deletion
- [High] mm/damon: missing charge_target_from reset when target has no regions --
commit 34b3a9a3bd49604e23a1fa56349694c2bc0db52b
Author: Liew Rui Yan <[email protected]>
mm/damon/core: fix unconditionally skip last region
This commit modifies the quota charge skipping logic to only reset charge_target_from and charge_addr_from when the last region is actually reached. It addresses a minor issue where the last region of a tracked target could be unconditionally skipped and reset even when it had not been processed yet.
Link https://lore.kernel.org/[email protected] open here
Quoted 5 lines
mm/damon/core.c +0 -0
index c86cac855b9b0..c459eae4bbaeb 100644
mm/damon/core.c static bool damos_skip_charged_region(struct damon_target *t, · line 892
Quoted 9 lines
mm/damon/core.c +1 -0
@@ -884,36 +884,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]
Does this logic stall the scheme indefinitely if charge_target_from points to a destroyed target?
If a target is removed during a sysfs commit, damon_sysfs_set_targets() dynamically destroys it:
mm/damon/sysfs.c:damon_sysfs_set_targets() {
[ ... ]
		} else {
			if (damon_target_has_pid(ctx))
				put_pid(t->pid);
			damon_destroy_target(t);
		}
[ ... ]
}
However, damon_sysfs_set_schemes() merely updates existing schemes in-place and fails to clear the quota->charge_target_from pointer if it points to the deleted target.
Because this saved target was deleted and removed from the active target list, would this condition evaluate to true for all remaining targets, permanently skipping all regions and never reaching the logic to clear the dangling pointer?
[Severity: High]
Does this also cause a stall if a target loses all of its regions?
If quota->charge_target_from points to a target that loses all its regions (for example, if all VMAs are unmapped and the task dies), damon_for_each_region() will execute zero times for that target:
mm/damon/core.c:kdamond_apply_schemes() {
[ ... ]
	damon_for_each_target(t, c) {
		damon_for_each_region(r, t)
			damon_do_apply_schemes(c, t, r);
	}

Quoted 7 lines

[ ... ]
}
Would this bypass the invocation of damos_skip_charged_region() entirely for the charged target, preventing charge_target_from from being cleared, and causing all subsequent targets to be skipped indefinitely because they do not match the uncleared charge_target_from pointer?

Pre-existing issue that also exist in the mainline. Hence no blocker of this backport. I will separately work on the issue.

Quoted 4 lines

Signature · 2 lines

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

Thanks,
SJ

Sasha Levin

maintainer Applied

2026-10-11 16:48 UTC #20 lore raw

Re: [PATCH 6.6.y v3 0/2] mm/damon/core: fix last region handling of damos_skip_charged_regions()

In reply to SJ Park

Quoted 3 lines

Patch 1 is a dependency of patch 2. Without it, patch 2 introduces out-of-bounds memory access bug that was found by Sashiko. Patch 2 fixes a bug that categorized to be backported to stable@.

Queued the series for 6.6, thanks.

Signature · 2 lines

Thanks,
Sasha