Re: [PATCH 2/2] mm/damon/reclaim: fix min_age_region truncation from unsigned long to unsigned int

[SeongJae Park]

On Thu, 19 Mar 2026 16:16:20 +0000 Josh Law <objecting@xxxxxxxxxxxxx> wrote:

> min_age / aggr_interval can exceed UINT_MAX with extreme but valid
> module parameter values, silently truncating min_age_region and
> lowering the cold-page age threshold below what the user intended.
> Clamp the result to UINT_MAX to match the field's type.

I believe the issue is not making real issue in real life.  But fixing this
makes the code cleaner to read, so I think this is better to have.  Thank you!

> 
> Signed-off-by: Josh Law <objecting@xxxxxxxxxxxxx>

Reviewed-by: SeongJae Park <sj@xxxxxxxxxx>

> ---
>  mm/damon/reclaim.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 9b55df304e51..303c64a56073 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
> @@ -170,8 +170,8 @@ static struct damos *damon_reclaim_new_scheme(void)
>  		.min_nr_accesses = 0,
>  		.max_nr_accesses = 0,
>  		/* for min_age or more micro-seconds */
> -		.min_age_region = min_age /
> -			damon_reclaim_mon_attrs.aggr_interval,
> +		.min_age_region = min_t(unsigned long, min_age /
> +			damon_reclaim_mon_attrs.aggr_interval, UINT_MAX),
>  		.max_age_region = UINT_MAX,
>  	};

Sashiko comment.
# review url: https://sashiko.dev/#/patchset/20260319161620.189392-3-objecting@xxxxxxxxxxxxx

: Could direct access to damon_reclaim_mon_attrs.aggr_interval result in a
: division by zero? If a user writes 0 to the module parameter concurrently via
: sysfs between the validation step and this division, it appears the interval
: could be zero here.

Makes sense, but orthogonal to this patch.  I will work on this.

: 
: Also, since both min_age_region and max_age_region are now clamped or set to
: UINT_MAX, the target matching condition becomes exactly UINT_MAX. In
: mm/damon/core.c (kdamond_merge_regions), a region's age increments on each
: interval via r->age++ without bounds checking.
: 
: Does this mean a region's age will hit UINT_MAX, match for a single interval,
: and then wrap around to 0, effectively defeating the "reclaim if older than
: X" logic?

Good point.  But such case may be too rare to consider...?  Anyway irrelevant
to this patch.  I will take more time to think about this.

: 
: Finally, should similar clamping be applied to other DAMON modules? For
: example, in mm/damon/lru_sort.c, cold_thres and hot_thres are computed from
: unsigned long parameters but typed as unsigned int. Similarly, in
: mm/damon/sysfs-schemes.c, damon_sysfs_mk_scheme assigns unsigned long
: boundaries to the damos_access_pattern fields, which could also result in
: silent truncation.

Yes, similar issue may exist in DAMON_LRU_SORT.  But definitely it is
orthogonal to this patch.  Also, as I mentioned above, I think this change is
good to have but not really fixing real critical issue.  If a nice person send
a patch for this, I'd appreciate.


Thanks,
SJ