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

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

[SJ Park]

From: Liew Rui Yan <aethernet65535@gmail.com>

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/20260908134739.96919-1-sj@kernel.org
Fixes: 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: <stable@vger.kernel.org> # v5.16.x
(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 32737cc5f877b..802005d9186e8 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1322,19 +1322,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) {
@@ -1348,11 +1346,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,
-- 
2.47.3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] damon: OOB write and list corruption when damon_split_region_at() fails
- [High] damon: permanent skipping of targets due to charge_target_from state leak
--

commit 035ce5241b6b256409f7ad3d4de6c01dbe7e8563
Author: Liew Rui Yan <aethernet65535@gmail.com>
mm/damon/core: fix unconditionally skip last region

This patch modifies the skip logic in DAMON to only reset the
charge_target_from state when the last region is reached. It also updates
the function to skip the region only if it is applied or cannot be split,
rather than unconditionally skipping the last region.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 32737cc5f877b..802005d9186e8 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -1348,11 +1346,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;

[Severity: Critical]
Could this lead to out-of-bounds writes and list corruption if
damon_split_region_at() fails?

This patch removes the early exit condition for the last region, exposing it
to damon_split_region_at(). If memory allocation fails, the split function
returns silently without modifying the list:

mm/damon/core.c:damon_split_region_at() {
	new = damon_new_region(r->ar.start + sz_r, r->ar.end);
	if (!new)
		return;
...
}

The pointer is then unconditionally advanced with r = damon_next_region(r).
If r was the last region, damon_next_region() wraps around to the list
head (&t->regions_list), generating an invalid struct damon_region pointer
pointing outside valid regions. Subsequent writes to this pointer will
corrupt the target sibling list integrity.

> +			skip = false;
>  		}
> +	}
> +out:
> +	if (r == damon_last_region(t)) {

[Severity: High]
Does the early return inside the region size check bypass this cleanup logic?

If sz_to_skip evaluates to 0 and the region size is <= DAMON_MIN_REGION,
the code hits an early return:

mm/damon/core.c:damos_skip_charged_region() {
...
		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;
...
}

Because this early return bypasses the new out label,
quota->charge_target_from remains populated if this occurs on the last
region. For all subsequent targets, the check
if (t != quota->charge_target_from) evaluates to true and indefinitely skips
those regions, causing targets to be starved.

>  		quota->charge_target_from = NULL;
>  		quota->charge_addr_from = 0;
>  	}
> -	return false;
> +	return skip;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930092829.45885-1-sj@kernel.org?part=1

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

[SJ Park]

On Wed, 30 Sep 2026 09:53:34 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] damon: OOB write and list corruption when damon_split_region_at() fails
> - [High] damon: permanent skipping of targets due to charge_target_from state leak
> --
> 
> commit 035ce5241b6b256409f7ad3d4de6c01dbe7e8563
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> mm/damon/core: fix unconditionally skip last region
> 
> This patch modifies the skip logic in DAMON to only reset the
> charge_target_from state when the last region is reached. It also updates
> the function to skip the region only if it is applied or cannot be split,
> rather than unconditionally skipping the last region.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 32737cc5f877b..802005d9186e8 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -1348,11 +1346,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;
> 
> [Severity: Critical]
> Could this lead to out-of-bounds writes and list corruption if
> damon_split_region_at() fails?
> 
> This patch removes the early exit condition for the last region, exposing it
> to damon_split_region_at(). If memory allocation fails, the split function
> returns silently without modifying the list:
> 
> mm/damon/core.c:damon_split_region_at() {
> 	new = damon_new_region(r->ar.start + sz_r, r->ar.end);
> 	if (!new)
> 		return;
> ...
> }
> 
> The pointer is then unconditionally advanced with r = damon_next_region(r).
> If r was the last region, damon_next_region() wraps around to the list
> head (&t->regions_list), generating an invalid struct damon_region pointer
> pointing outside valid regions. Subsequent writes to this pointer will
> corrupt the target sibling list integrity.

This is a pre-existing issue.  I will separately work on the fix.

> 
> > +			skip = false;
> >  		}
> > +	}
> > +out:
> > +	if (r = damon_last_region(t)) {
> 
> [Severity: High]
> Does the early return inside the region size check bypass this cleanup logic?
> 
> If sz_to_skip evaluates to 0 and the region size is <= DAMON_MIN_REGION,
> the code hits an early return:
> 
> mm/damon/core.c:damos_skip_charged_region() {
> ...
> 		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;
> ...
> }
> 
> Because this early return bypasses the new out label,
> quota->charge_target_from remains populated if this occurs on the last
> region. For all subsequent targets, the check
> if (t != quota->charge_target_from) evaluates to true and indefinitely skips
> those regions, causing targets to be starved.

Good catch.  This is a mistake in my backporting.  I will send v2.

> 
> >  		quota->charge_target_from = NULL;
> >  		quota->charge_addr_from = 0;
> >  	}
> > -	return false;
> > +	return skip;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260930092829.45885-1-sj@kernel.org?part=1
> 


Thanks,
SJ

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

[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/20260930100106.52943-1-sj@kernel.org


Thanks,
SJ

[...]

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

[SJ Park]

From: Liew Rui Yan <aethernet65535@gmail.com>

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/20260908134739.96919-1-sj@kernel.org
Fixes: 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: <stable@vger.kernel.org> # v5.16.x
(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from v1
- v1: https://lore.kernel.org/20260930092829.45885-1-sj@kernel.org
- Add missed change for damon_sz_region(r) <= min_region_sz case.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 32737cc5f877b..cc7250f0936af 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1322,37 +1322,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,
-- 
2.47.3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: OOB dereference in damon_do_apply_schemes() on split failure
--

commit 61e0ff2e7385bb544355d1f3aa3598f7f8e9e1d0
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/core: fix unconditionally skip last region

This commit changes the logic in damos_skip_charged_region() to only reset
charge_{target,addr}_from when the last region is reached. This is intended to
ensure the last region is not skipped if it has not been fully processed.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 32737cc5f877b..cc7250f0936af 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -1322,37 +1322,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)) {

[Severity: High]
If damon_split_region_at() fails to allocate memory, it returns silently and
the region remains unmodified. If r happens to be the last region in the list,
could damon_next_region(r) evaluate past the list boundary and return an
invalid pointer?

Since this invalid pointer is assigned to *rp and the function continues to
the out label, could it fail this (r == damon_last_region(t)) check, bypass
the intended quota reset, and return false?

If so, could the caller damon_do_apply_schemes() then receive this invalid
pointer and pass it to damos_valid_target():

mm/damon/core.c:damon_do_apply_schemes() {
    ...
		if (damos_skip_charged_region(t, &r, s))
			continue;

		if (!damos_valid_target(c, t, r, s))
			continue;
    ...
}

causing it to dereference garbage data (such as r->ar.start and r->ar.end) and
potentially lead to an out-of-bounds memory access?

>  		quota->charge_target_from = NULL;
>  		quota->charge_addr_from = 0;
>  	}
> -	return false;
> +	return skip;
>  }
>  
>  static void damos_update_stat(struct damos *s,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930101459.56041-1-sj@kernel.org?part=1

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

[SJ Park]

On Wed, 30 Sep 2026 10:26:49 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm: OOB dereference in damon_do_apply_schemes() on split failure
> --
> 
> commit 61e0ff2e7385bb544355d1f3aa3598f7f8e9e1d0
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix unconditionally skip last region
> 
> This commit changes the logic in damos_skip_charged_region() to only reset
> charge_{target,addr}_from when the last region is reached. This is intended to
> ensure the last region is not skipped if it has not been fully processed.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 32737cc5f877b..cc7250f0936af 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -1322,37 +1322,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)) {
> 
> [Severity: High]
> If damon_split_region_at() fails to allocate memory, it returns silently and
> the region remains unmodified. If r happens to be the last region in the list,
> could damon_next_region(r) evaluate past the list boundary and return an
> invalid pointer?
> 
> Since this invalid pointer is assigned to *rp and the function continues to
> the out label, could it fail this (r == damon_last_region(t)) check, bypass
> the intended quota reset, and return false?
> 
> If so, could the caller damon_do_apply_schemes() then receive this invalid
> pointer and pass it to damos_valid_target():
> 
> mm/damon/core.c:damon_do_apply_schemes() {
>     ...
> 		if (damos_skip_charged_region(t, &r, s))
> 			continue;
> 
> 		if (!damos_valid_target(c, t, r, s))
> 			continue;
>     ...
> }
> 
> causing it to dereference garbage data (such as r->ar.start and r->ar.end) and
> potentially lead to an out-of-bounds memory access?

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

[1] https://lore.kernel.org/20260930102333.58847-1-sj@kernel.org

> 
> >  		quota->charge_target_from = NULL;
> >  		quota->charge_addr_from = 0;
> >  	}
> > -	return false;
> > +	return skip;
> >  }
> >  
> >  static void damos_update_stat(struct damos *s,
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260930101459.56041-1-sj@kernel.org?part=1


Thanks,
SJ

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

[Sasha Levin]

> 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.

The 6.18 backport that is already queued has the same split failure
exposure, so it will need that fix too.

-- 
Thanks,
Sasha

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

[SJ Park]

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/20260930101459.56041-1-sj@kernel.org
- Fix out-of-bounds memory access bug by adding patch 1.
Changes from v1
- v1: https://lore.kernel.org/20260930092829.45885-1-sj@kernel.org
- Add missed change for damon_sz_region(r) <= min_region_sz case.

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: da5a052a482924abb7ca43f3cc70622b3033f5cb
-- 
2.47.3

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

[SJ Park]

From: SeongJae Park <sj@kernel.org>

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/20260227170623.95384-3-sj@kernel.org
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 1745ccbd2907db2bdaa843e4abccde4fdaccbe5d)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 32737cc5f877..76f23c1d2f56 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1310,16 +1310,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;
 
@@ -1346,8 +1347,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;
@@ -1514,7 +1514,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))
@@ -1727,7 +1727,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;
@@ -1749,7 +1749,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);
 	}
 
-- 
2.47.3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: damon: double application of DAMOS schemes due to non-safe region walk
--

commit 7c60a2f66acad38f06b09b2a9e484519168800bc
Author: SeongJae Park <sj@kernel.org>

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

This commit changes kdamond_apply_schemes() to use the non-safe
damon_for_each_region() iterator instead of damon_for_each_region_safe().
It also drops the double-pointer workaround in damos_skip_charged_region()
to let newly split regions be seen on the next iteration of the region walk.

Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 32737cc5f877b..76f23c1d2f56b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1749,7 +1749,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: High]
Does this change cause earlier schemes to double-apply their actions when a
subsequent scheme splits the region?

In kdamond_apply_schemes(), changing to the non-safe iterator means the loop
dynamically visits newly created fragments on the very next iteration.

When damon_do_apply_schemes() runs, it evaluates all schemes sequentially on
the region:

mm/damon/core.c:damon_do_apply_schemes() {
	...
	damon_for_each_scheme(s, c) {
		...
		if (damos_skip_charged_region(t, r, s))
			continue;
		...
		damos_apply_scheme(c, t, r, s);
	}
}

If an earlier scheme evaluates the region, it may apply its action and update
its statistics for the full size of the region.

If a subsequent scheme then evaluates the same region and splits it (for
example via damos_skip_charged_region()), it shrinks the original region and
inserts a new region immediately after it.

Because damon_for_each_region() is no longer safe against modifications to the
list, the next iteration of the outer loop will process the newly inserted
region across all schemes.

Since the newly inserted region retains the exact properties (age, access
counts) of the original region, will the earlier scheme re-evaluate it and
apply its action a second time, double-charging its statistics and applying
redundant actions to the memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009123651.6585-1-sj@kernel.org?part=1

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

[SJ Park]

From: Liew Rui Yan <aethernet65535@gmail.com>

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/20260908134739.96919-1-sj@kernel.org
Fixes: 50585192bc2e ("mm/damon/schemes: skip already charged targets and regions")
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Cc: <stable@vger.kernel.org> # v5.16.x
(cherry picked from commit b3723b596b548c837a766aae3553c14a7b15af2b)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 76f23c1d2f56..dab9669de04b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1323,36 +1323,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,
-- 
2.47.3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm/damon: dangling charge_target_from pointer permanently freezes schemes
--

commit a9baf64370898b2dec04fa2f17e4061166d6d27a
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon/core: fix unconditionally skip last region

This commit modifies damos_skip_charged_region() to clear the quota's
charge_target_from pointer only when the last region of a target is
reached, rather than clearing it unconditionally during previous charge
checks.

Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1323,36 +1323,39 @@ static bool damos_skip_charged_region(struct damon_target *t,
>  		struct damon_region *r, struct damos *s)
>  {
>  	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;

[ ... ]

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

[Severity: High]
If the target tracked by quota->charge_target_from is dynamically
destroyed via sysfs, or if it loses all of its regions, will this
clearing logic ever be reached?

If the saved target is removed or has zero regions, the region loop will
be empty, meaning r == damon_last_region(t) is never evaluated for
that target.

Since charge_target_from is never cleared in these scenarios, wouldn't
this cause the condition earlier in this function:

        if (t != quota->charge_target_from)
                return true;

to remain true indefinitely for all other valid targets, permanently
freezing the scheme? Additionally, if the target is freed, does this
leave a dangling pointer comparison for future checks?

> -	return false;
> +	return skip;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009123651.6585-1-sj@kernel.org?part=2

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

[SJ Park]

On Fri, 09 Oct 2026 13:06:32 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: dangling charge_target_from pointer permanently freezes schemes
> --
> 
> commit a9baf64370898b2dec04fa2f17e4061166d6d27a
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/core: fix unconditionally skip last region
> 
> This commit modifies damos_skip_charged_region() to clear the quota's
> charge_target_from pointer only when the last region of a target is
> reached, rather than clearing it unconditionally during previous charge
> checks.
> 
> Link: https://lore.kernel.org/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1323,36 +1323,39 @@ static bool damos_skip_charged_region(struct damon_target *t,
> >  		struct damon_region *r, struct damos *s)
> >  {
> >  	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;
> 
> [ ... ]
> 
> > +	}
> > +out:
> > +	if (r = damon_last_region(t)) {
> >  		quota->charge_target_from = NULL;
> >  		quota->charge_addr_from = 0;
> >  	}
> 
> [Severity: High]
> If the target tracked by quota->charge_target_from is dynamically
> destroyed via sysfs, or if it loses all of its regions, will this
> clearing logic ever be reached?

Good finding.  But this is pre-existing issue that also exist in the mainline
tree.  So not a blocker of this patch in my opinion.  I will separately working
on this.

> 
> If the saved target is removed or has zero regions, the region loop will
> be empty, meaning r = damon_last_region(t) is never evaluated for
> that target.
> 
> Since charge_target_from is never cleared in these scenarios, wouldn't
> this cause the condition earlier in this function:
> 
>         if (t != quota->charge_target_from)
>                 return true;
> 
> to remain true indefinitely for all other valid targets, permanently
> freezing the scheme? Additionally, if the target is freed, does this
> leave a dangling pointer comparison for future checks?
> 
> > -	return false;
> > +	return skip;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261009123651.6585-1-sj@kernel.org?part=2
> 


Thanks,
SJ