Re: [RFC PATCH v1.1 1/7] mm/damon/core: remove obsolete comment for nr_to_bp() divide-by-zero

[SJ Park <sj@kernel.org>]

On Tue, 07 Jul 2026 13:57:50 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Removing the `!attrs.aggr_interval` skip guard causes `damon_test_nr_accesses_to_accesses_bp` to fail deterministically on 32-bit architectures.
> --
> 
> --- Patch [1]: [RFC PATCH v1.1 1/7] mm/damon/core: remove obsolete comment for nr_to_bp() divide-by-zero ---
> commit a31fcddef9e05dc323b42f64143ffe58dc702dee
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: remove obsolete comment for nr_to_bp() divide-by-zero
> 
> This commit removes obsolete comments about division-by-zero in
> damon_nr_accesses_to_accesses_bp() and its related test code. A previous
> commit modified the underlying calculation to always return a non-zero value,
> preventing the division-by-zero issue.
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 0124f83b39b83..224a3ecaa0701 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> [ ... ]
> > @@ -582,18 +582,6 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
> >  		.aggr_interval = ((unsigned long)UINT_MAX + 1) * 10
> >  	};
> >  
> > -	/*
> > -	 * In some cases such as 32bit architectures where UINT_MAX is
> > -	 * ULONG_MAX, attrs.aggr_interval becomes zero.  Calling
> > -	 * damon_nr_accesses_to_accesses_bp() in the case will cause
> > -	 * divide-by-zero.  Such case is prohibited in normal execution since
> > -	 * the caution is documented on the comment for the function, and
> > -	 * damon_update_monitoring_results() does the check.  Skip the test in
> > -	 * the case.
> > -	 */
> > -	if (!attrs.aggr_interval)
> > -		kunit_skip(test, "aggr_interval is zero.");
> > -
> >  	KUNIT_EXPECT_EQ(test, damon_nr_accesses_to_accesses_bp(123, &attrs), 0);
> 
> [Severity: Medium]
> Will removing this skip guard cause a deterministic test failure on 32-bit
> architectures?
> 
> When the test runs on a 32-bit architecture, attrs.aggr_interval is initialized
> to ((unsigned long)UINT_MAX + 1) * 10, which overflows to 0. With the division
> by zero fix in place, damon_max_nr_accesses(&attrs) will return 1 instead of 0.
> 
> This causes damon_nr_accesses_to_accesses_bp(123, &attrs) to evaluate to
> (123 * 10000) / 1 = 1230000. Since the test unconditionally expects the result
> to be 0, the KUNIT_EXPECT_EQ assertion will fail.

Good point.  Actually, this test was only for division by zero, and now it
seems it is just broken.  Maybe a better way is testing if
damon_max_nr_accesses() returns zero or not in the extreme cases.

I will simply drop the entire test from the next revision.

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

Thanks,
SJ