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

[SeongJae Park <sj@kernel.org>]

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

[...]