Re: [PATCH 6.18.y 1/5] mm/damon/core: skip applying scheme if region split for quota fails

[SJ Park <sj@kernel.org>]

On Sun, 08 Nov 2026 19:35:07 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Low] mm/damon: damos_apply_scheme() inflates stats on region split
>   failure
> It also noticed 1 related pre-existing issue this patch does not
> introduce:
> - [High] mm/damon/core: total_charged_ns overflow in
>   damos_apply_scheme() [1]
> --
> 
> commit 9174bd7b58d2e2acd57046175fb6abb05138bbb9
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: skip applying scheme if region split for quota fails
> 
> This commit updates damon_split_region_at() to return an error code on
> allocation failure and skips applying a scheme in damos_apply_scheme()
> when splitting a region for quota enforcement fails.
> 
> Link: https://lore.kernel.org/20260901131850.98037-1-sj@kernel.org
> Link: https://lore.kernel.org/20260901131850.98037-2-sj@kernel.org
> Link: https://lore.kernel.org/20260718171523.87547-1-sj@kernel.org [1]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 8fa788b0d709c..4c536a9b958e4 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -2000,7 +2000,8 @@ static void damos_apply_scheme(struct damon_ctx *c, struct damon_target *t,
> >  					c->min_sz_region);
> >  			if (!sz)
> >  				goto update_stat;
> > -			damon_split_region_at(t, r, sz);
> > +			if (damon_split_region_at(t, r, sz))
> > +				goto update_stat;
> >  		}
> >  		if (damos_filter_out(c, t, r, s))
> >  			return;
> 
> [Severity: Low]
> Does jumping to update_stat when damon_split_region_at() fails inflate
> s->stat.nr_tried and s->stat.sz_tried?
> 
> At this point sz is the non-zero aligned remaining quota size.  Jumping to
> update_stat calls damos_update_stat(s, sz, 0, 0), which increments
> s->stat.nr_tried and adds sz to s->stat.sz_tried even though
> damos_filter_out() and c->ops.apply_scheme() were skipped.  Because
> quota->charged_sz is not updated on failure, subsequent oversized regions
> in the walk that also fail to split will repeatedly add sz to
> s->stat.sz_tried.

User impact is trivial and the same behavior exists in the mainline.  No
blocker.

> 
> [Severity: High]
> This problem wasn't introduced by this patch, but can
> quota->total_charged_ns overflow on 32-bit architectures [1] in
> damos_apply_scheme()?

Known low priority issue.  No blocker for this patch.

> 
> struct damos_quota defines total_charged_ns as unsigned long, which wraps
> around zero after roughly 4.29 seconds of accumulated scheme execution on
> 32-bit platforms:
> 
> include/linux/damon.h:struct damos_quota {
>     ...
> 	unsigned long total_charged_ns;
>     ...
> };
> 
> mm/damon/core.c:damos_apply_scheme() {
>     ...
> 		quota->total_charged_ns += timespec64_to_ns(&end) -
> 			timespec64_to_ns(&begin);
>     ...
> }
> 
> mm/damon/core.c:damos_set_effective_quota() {
>     ...
> 		if (quota->total_charged_ns)
> 			throughput = mult_frac(quota->total_charged_sz, 1000000,
> 							quota->total_charged_ns);
>     ...
> }
> 
> When total_charged_ns wraps to a small value, damos_set_effective_quota()
> computes an inflated throughput and effective size quota (quota->esz),
> causing DAMOS to process far more memory than allowed by the time quota.
> 
> [1] https://sashiko.dev/#/bug/linux-c8969436-aa48-48c6-9f46-a12a64361494
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261108192221.114362-1-sj@kernel.org?part=1
> 


Thanks,
SJ