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

[PATCH 6.6.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 0713fda22682c..5ede1733369f5 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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,
-- 
2.47.3

Re: [PATCH 6.6.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:
- [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 <aethernet65535@gmail.com>

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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 0713fda22682c..5ede1733369f5 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

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

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

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

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

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

[SJ Park]

On Wed, 30 Sep 2026 09:54:41 +0000 sashiko-bot@kernel.org 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 <aethernet65535@gmail.com>
> 
> 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/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 0713fda22682c..5ede1733369f5 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> >  		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.

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

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

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

> 
> > +			skip = false;
> >  		}
> > +	}
> > +out:
> > +	if (r == damon_last_region(t)) {
> >  		quota->charge_target_from = NULL;
> >  		quota->charge_addr_from = 0;
> >  	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260930093707.48739-1-sj@kernel.org?part=1


Thanks,
SJ

Re: [PATCH 6.6.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/20260930100637.53340-1-sj@kernel.org


Thanks,
SJ

[...]

[PATCH 6.6.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/20260930093707.48739-1-sj@kernel.org
- Add missed damon_sz_region(r) <= min_region_sz case change.

 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 0713fda22682c..c31683c12dd48 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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,
-- 
2.47.3

Re: [PATCH 6.6.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/damon: list head type confusion and memory corruption on allocation failure
--

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

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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 0713fda22682c..c31683c12dd48 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -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 (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;

[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?

>  		}
> +	}
> +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?

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

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

[SJ Park]

On Wed, 30 Sep 2026 10:31:50 +0000 sashiko-bot@kernel.org 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 <aethernet65535@gmail.com>
> 
> 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/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 0713fda22682c..c31683c12dd48 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -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 (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;
> 
> [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?
> 
> >  		}
> > +	}
> > +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?

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

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


Thanks,
SJ

Re: [PATCH 6.6.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.

-- 
Thanks,
Sasha

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

[SJ Park]

On Fri,  2 Oct 2026 10:19:50 -0400 Sasha Levin <sashal@kernel.org> wrote:

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

[...]

[PATCH 6.6.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/20260930101853.58786-1-sj@kernel.org
- Fix out-of-bounds memory access bug by adding patch 1.
Changes from v1
- v1: https://lore.kernel.org/20260930093707.48739-1-sj@kernel.org
- 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
-- 
2.47.3

[PATCH 6.6.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 fe91b296f4c6..c86cac855b9b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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);
 	}
 
-- 
2.47.3

[PATCH 6.6.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 c86cac855b9b..c459eae4bbae 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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,
-- 
2.47.3

Re: [PATCH 6.6.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:
- [Medium] mm: damon: double processing of split regions by earlier schemes
--

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

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/20260227170623.95384-3-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index fe91b296f4c66..c86cac855b9b0 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -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);

[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?

>  	}
>

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

Re: [PATCH 6.6.y v3 2/2] 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:
- [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 <aethernet65535@gmail.com>

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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index c86cac855b9b0..c459eae4bbaeb 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -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?

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

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

[SJ Park]

On Fri, 09 Oct 2026 14:02:33 +0000 sashiko-bot@kernel.org 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 <sj@kernel.org>
> 
> 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/20260227170623.95384-3-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index fe91b296f4c66..c86cac855b9b0 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -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);
> 
> [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?

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.

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


Thanks,
SJ

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

[SJ Park]

On Fri, 09 Oct 2026 14:05:09 +0000 sashiko-bot@kernel.org 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 <aethernet65535@gmail.com>
> 
> 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/20260908134739.96919-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index c86cac855b9b0..c459eae4bbaeb 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -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?

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

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


Thanks,
SJ