Re: (sashiko review) [RFC PATCH v3 01/10] mm/damon/core: handle <min_region_sz remaining quota as empty

SeongJae Park
2026-04-07T14:12:57Z

TL; DR: sashiko made a good suggestion. I will address that in the next
revision.

Forwarding full sashiko review in a reply format with my
inline comments below, for sharing details of my view and
doing followup discussions via mails if needed.

> # review url: https://sashiko.dev/#/patchset/20260407010536.83603-2-sj@kernel.org
> # start of sashiko.dev inline review
> commit 3657261e42ea1ce93233b322fabf0d2c5c335a35
> Author: SeongJae Park <sj@kernel.org>
>
> mm/damon/core: handle <min_region_sz remaining quota as empty
>
> This commit updates the DAMON quota checks to treat a remaining quota of less
> than min_region_sz as fully charged. It introduces a new helper function,
> damos_quota_is_full(), and replaces the existing explicit quota checks with it.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3bc7a2bbfe7de..c29f70bcce894 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -2041,6 +2041,14 @@ static void damos_walk_cancel(struct damon_ctx *ctx)
> > mutex_unlock(&ctx->walk_control_lock);
> > }
> >
> > +static bool damos_quota_is_full(struct damos_quota *quota,
> > + unsigned long min_region_sz)
> > +{
> > + if (!damos_quota_is_set(quota))
> > + return false;
> > + return quota->charged_sz + min_region_sz > quota->esz;
>
> Can quota->charged_sz + min_region_sz overflow here?
>
> Since both variables are unsigned long, if min_region_sz is configured to a
> very large value via sysfs, or on 32-bit systems where memory regions can
> naturally be a large fraction of ULONG_MAX, the sum could wrap around.
>
> If it wraps around, it would evaluate to a value smaller than quota->esz.
> This would cause damos_quota_is_full() to incorrectly return false even when
> the quota is fully exhausted, bypassing the DAMON action quota limits.
>
> Could this be written using subtraction to prevent overflow?
>
> return quota->charged_sz >= quota->esz ||
> quota->esz - quota->charged_sz < min_region_sz;

Good suggestion, I will do so in the next revision.

>
> [ ... ]
>
>
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260407010536.83603-2-sj@kernel.org

Thanks,
SJ

# hkml [1] generated a draft of this mail. You can regenerate
# this using below command:
#
# hkml patch sashiko_dev --for_forwarding \
# 20260407010536.83603-2-sj@kernel.org
#
# [1] https://github.com/sjp38/hackermail