Re: [PATCH v3 2/2] mm/damon/core: eliminate hot-path integer division in damon_max_nr_accesses()

[Josh Law <objecting@objecting.org>]

On 24 March 2026 07:19:08 GMT, SeongJae Park <sj@kernel.org> wrote:
>On Sun, 22 Mar 2026 21:43:25 +0000 Josh Law <objecting@objecting.org> wrote:
>
>> Hardware integer division is slow. The function damon_max_nr_accesses(),
>> which is called very frequently (e.g., once per region per sample
>> interval inside damon_update_region_access_rate), performs an integer
>> division: attrs->aggr_interval / attrs->sample_interval.
>> 
>> However, the struct damon_attrs already caches this exact ratio in the
>> internal field aggr_samples (since earlier commits). We can eliminate
>> the hardware division in the hot path by simply returning aggr_samples.
>> 
>> This significantly reduces the CPU cycle overhead of updating the access
>> rates for thousands of regions.
>> 
>> Signed-off-by: Josh Law <objecting@objecting.org>
>> ---
>>  include/linux/damon.h | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/include/linux/damon.h b/include/linux/damon.h
>> index 6bd71546f7b2..438fe6f3eab4 100644
>> --- a/include/linux/damon.h
>> +++ b/include/linux/damon.h
>> @@ -960,8 +960,7 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
>>  static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
>>  {
>>  	/* {aggr,sample}_interval are unsigned long, hence could overflow */
>> -	return min(attrs->aggr_interval / attrs->sample_interval,
>> -			(unsigned long)UINT_MAX);
>> +	return min_t(unsigned long, attrs->aggr_samples, UINT_MAX);
>>  }
>
>I just found this patch causes below divide-by-zero when
>tools/testing/selftets/damon/sysfs.py is executed.
>
>'''
>[   42.462039] Oops: divide error: 0000 [#1] SMP NOPTI
>[   42.463673] CPU: 4 UID: 0 PID: 2044 Comm: kdamond.0 Not tainted 7.0.0-rc4-mm-new-damon+ #354 PREEMPT(full)
>[   42.465193] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
>[   42.466590] RIP: 0010:damon_set_attrs (mm/damon/core.c:769 (discriminator 1) mm/damon/core.c:775 (discriminator 1) mm/damon/core.c:786 (discriminator 1) mm/damon/core.c:827 (discriminator 1) mm/damon/core.c:897 (discriminator 1))
>[   42.467287] Code: 48 39 c5 0f 84 dd 00 00 00 41 bb 59 17 b7 d1 48 8b 43 10 4c 8d 43 10 48 8d 48 e0 49 39 c0 75 5b e9 b0 00 00 00 8b 41 18 31 d2 <41> f7 f6 41 89 c5 69 c2 10 27 00 00 31 d2 45 69 ed 10 27 00 00 41
>All code
>========
>   0:   48 39 c5                cmp    %rax,%rbp
>   3:   0f 84 dd 00 00 00       je     0xe6
>   9:   41 bb 59 17 b7 d1       mov    $0xd1b71759,%r11d
>   f:   48 8b 43 10             mov    0x10(%rbx),%rax
>  13:   4c 8d 43 10             lea    0x10(%rbx),%r8
>  17:   48 8d 48 e0             lea    -0x20(%rax),%rcx
>  1b:   49 39 c0                cmp    %rax,%r8
>  1e:   75 5b                   jne    0x7b
>  20:   e9 b0 00 00 00          jmp    0xd5
>  25:   8b 41 18                mov    0x18(%rcx),%eax
>  28:   31 d2                   xor    %edx,%edx
>  2a:*  41 f7 f6                div    %r14d            <-- trapping instruction
>  2d:   41 89 c5                mov    %eax,%r13d
>  30:   69 c2 10 27 00 00       imul   $0x2710,%edx,%eax
>  36:   31 d2                   xor    %edx,%edx
>  38:   45 69 ed 10 27 00 00    imul   $0x2710,%r13d,%r13d
>  3f:   41                      rex.B
>
>Code starting with the faulting instruction
>===========================================
>   0:   41 f7 f6                div    %r14d
>   3:   41 89 c5                mov    %eax,%r13d
>   6:   69 c2 10 27 00 00       imul   $0x2710,%edx,%eax
>   c:   31 d2                   xor    %edx,%edx
>   e:   45 69 ed 10 27 00 00    imul   $0x2710,%r13d,%r13d
>  15:   41                      rex.B
>[   42.470046] RSP: 0018:ffffd25c4586bcb0 EFLAGS: 00010246
>[   42.470818] RAX: 0000000000000000 RBX: ffff891346919400 RCX: ffff8913502dd040
>[   42.471923] RDX: 0000000000000000 RSI: ffff891348527600 RDI: ffff891344d94400
>[   42.472972] RBP: ffff891344d94598 R08: ffff891346919410 R09: 0000000000000000
>[   42.474028] R10: 0000000000000000 R11: 00000000d1b71759 R12: 0000000000000014
>[   42.475104] R13: ffff891348527778 R14: 0000000000000000 R15: ffff891348527798
>[   42.476191] FS:  0000000000000000(0000) GS:ffff89149efd8000(0000) knlGS:0000000000000000
>[   42.477375] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>[   42.478235] CR2: 000000003a84f080 CR3: 000000004a824000 CR4: 00000000000006f0
>[   42.479291] Call Trace:
>[   42.479670]  <TASK>
>[   42.480044]  damon_commit_ctx (mm/damon/core.c:1538)
>[   42.480660]  damon_sysfs_commit_input (mm/damon/sysfs.c:2153 mm/damon/sysfs.c:2181)
>[   42.481389]  kdamond_call (mm/damon/core.c:3186)
>[   42.482492]  kdamond_fn (mm/damon/core.c:3428)
>[   42.483041]  ? kthread_affine_node (kernel/kthread.c:377)
>[   42.483766]  ? kfree (include/linux/kmemleak.h:50 mm/slub.c:2610 mm/slub.c:6165 mm/slub.c:6483)
>[   42.484257]  ? __pfx_kdamond_fn (mm/damon/core.c:3368)
>[   42.484855]  ? __pfx_kdamond_fn (mm/damon/core.c:3368)
>[   42.485459]  kthread (kernel/kthread.c:436)
>[   42.485959]  ? __pfx_kthread (kernel/kthread.c:381)
>[   42.486524]  ret_from_fork (arch/x86/kernel/process.c:164)
>[   42.487105]  ? __pfx_kthread (kernel/kthread.c:381)
>[   42.487668]  ret_from_fork_asm (arch/x86/entry/entry_64.S:258)
>[   42.488304]  </TASK>
>'''
>
>That's because damon_commit_ctx() is called to a context that just generated
>using damon_new_ctx(), which doesn't set the aggr_samples.  After applying
>below change, the divide-by-zero is gone.
>
>'''
>--- a/mm/damon/core.c
>+++ b/mm/damon/core.c
>@@ -676,6 +676,7 @@ struct damon_ctx *damon_new_ctx(void)
>        ctx->attrs.sample_interval = 5 * 1000;
>        ctx->attrs.aggr_interval = 100 * 1000;
>        ctx->attrs.ops_update_interval = 60 * 1000 * 1000;
>+       ctx->attrs.aggr_samples = 20;
>
>        ctx->passed_sample_intervals = 0;
>        /* These will be set from kdamond_init_ctx() */
>'''
>
>Also, kunit crashes like below.
>
>'''
>$ ./tools/testing/kunit/kunit.py run --kunitconfig mm/damon/tests/
>[00:07:19] Configuring KUnit Kernel ...
>[00:07:19] Building KUnit Kernel ...
>Populating config with:
>$ make ARCH=um O=.kunit olddefconfig
>Building with:
>$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=8
>[00:08:11] Starting KUnit Kernel (1/1)...
>[00:08:11] ============================================================
>Running tests with:
>$ .kunit/linux kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
>[00:08:11] =================== damon (28 subtests) ====================
>[00:08:11] [PASSED] damon_test_target
>[00:08:11] [PASSED] damon_test_regions
>[00:08:11] [PASSED] damon_test_aggregate
>[00:08:11] [PASSED] damon_test_split_at
>[00:08:11] [PASSED] damon_test_merge_two
>[00:08:11] [PASSED] damon_test_merge_regions_of
>[00:08:11] [PASSED] damon_test_split_regions_of
>[00:08:11] [PASSED] damon_test_ops_registration
>[00:08:11] [PASSED] damon_test_set_regions
>[00:08:11] [ERROR] Test: damon: missing expected subtest!
>[00:08:11] Kernel panic - not syncing: Kernel mode signal 4
>'''
>
>It can run without the error after below changes are applied:
>
>'''
>--- a/mm/damon/tests/core-kunit.h
>+++ b/mm/damon/tests/core-kunit.h
>@@ -514,6 +514,8 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
>                .aggr_interval = ((unsigned long)UINT_MAX + 1) * 10
>        };
>
>+       attrs.aggr_samples = attrs.aggr_interval / attrs.sample_interval;
>+
>        /*
>         * In some cases such as 32bit architectures where UINT_MAX is
>         * ULONG_MAX, attrs.aggr_interval becomes zero.  Calling
>@@ -532,7 +534,8 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
> static void damon_test_update_monitoring_result(struct kunit *test)
> {
>        struct damon_attrs old_attrs = {
>-               .sample_interval = 10, .aggr_interval = 1000,};
>+               .sample_interval = 10, .aggr_interval = 1000,
>+               .aggr_samples = 100,};
>        struct damon_attrs new_attrs;
>        struct damon_region *r = damon_new_region(3, 7);
>
>@@ -544,19 +547,24 @@ static void damon_test_update_monitoring_result(struct kunit *test)
>        r->age = 20;
>
>        new_attrs = (struct damon_attrs){
>-               .sample_interval = 100, .aggr_interval = 10000,};
>+               .sample_interval = 100, .aggr_interval = 10000,
>+               .aggr_samples = 100,};
>        damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
>        KUNIT_EXPECT_EQ(test, r->nr_accesses, 15);
>        KUNIT_EXPECT_EQ(test, r->age, 2);
>
>        new_attrs = (struct damon_attrs){
>-               .sample_interval = 1, .aggr_interval = 1000};
>+               .sample_interval = 1, .aggr_interval = 1000,
>+               .aggr_samples = 1000,
>+       };
>        damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
>        KUNIT_EXPECT_EQ(test, r->nr_accesses, 150);
>        KUNIT_EXPECT_EQ(test, r->age, 2);
>
>        new_attrs = (struct damon_attrs){
>-               .sample_interval = 1, .aggr_interval = 100};
>+               .sample_interval = 1, .aggr_interval = 100,
>+               .aggr_samples = 100,
>+       };
>        damon_update_monitoring_result(r, &old_attrs, &new_attrs, false);
>        KUNIT_EXPECT_EQ(test, r->nr_accesses, 150);
>        KUNIT_EXPECT_EQ(test, r->age, 20);
>'''
>
>Josh, could you please also take a look if these fixups are sufficient?  And
>once the sufficient fixes are found from your side, could you please post a new
>version of this patch after applying the fixees?  Also, please drop my
>Reviewed-by: from the new version.  I will review it again.
>
>
>Thanks,
>SJ
>
>[...]


Aw, I didnt notice that, I'm so sorry, I'll fix this soon

V/R


Josh Law