linux-kernel - Re: [PATCH v2 0/2] mm/damon: Add damos

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20250730200239.60984-1-sj@kernel.org>
Date: Wed, 30 Jul 2025 13:02:39 -0700
From: SeongJae Park <sj@...nel.org>
To: Yueyang Pan <pyyjason@...il.com>
Cc: SeongJae Park <sj@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Usama Arif <usamaarif642@...il.com>,
	damon@...ts.linux.dev,
	linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/2] mm/damon: Add damos_stat support for vaddr

On Wed, 30 Jul 2025 10:19:54 -0700 Yueyang Pan <pyyjason@...il.com> wrote:

> Previously damos_stat only supoort paddr. This patch set adds support 
> for damos_stat for vaddr. Also all different types of filters are 
> supported. 
> 
> Functionality Test
> ==================
> I wrote a small test program which allocates 10GB of DRAM, use 
> madvise(MADV_HUGEPAGE) to convert the base pages to 2MB huge pages
> Then my program does the following things in order:
> 1. Write sequentially to the whole 10GB region
> 2. Read the first 5GB region sequentially for 10 times
> 3. Sleep 5s
> 4. Read the second 5GB region sequentially for 10 times
> 
> With a proper damon setting, we are expected to see df-passed to be 10GB
> and hot region move around with the read
> 
> $ # Start damon record
> $sudo ./damo/damo start "./my_test/test" --monitoring_intervals 100ms\
> 1s 2s

Above command is not really doing record, so the first line would better to be
updated, say, "Start DAMON" ?

Also, it is easier to read if you give prefix spaces for quoting commands as
well, to make it consistent with their outputs.  Fo for example, above could be
something like below.

    $ # Start DAMON
    $ sudo ./damo/damo start ...

> 
> $ # damon report
> $sudo ./damo/damo report access --snapshot_damos_filter allow \
> hugepage_size 2MiB 2MiB

Again, I'd prefer having prefix spaces for above commands consistent to the
following output.

Also, maybe the first comment could be more detailed, say, "Show
DAMON-generated access pattern snapshot" ?

>     heatmap:
>     # min/max temperatures: -600,000,000, 100,001,000, column size: 137.352 MiB
>     intervals: sample 100 ms aggr 1 s (max access hz 10)
>     # damos filters (df): reject none hugepage_size [2.000 MiB, 2.000 MiB]
>     df-pass:
>     # min/max temperatures: -400,000,000, 100,001,000, column size: 128.031 MiB
>     0   addr 85.373 TiB   size 745.555 MiB access 0 hz   age 6 s           df-passed 0 B
>     1   addr 127.608 TiB  size 877.664 MiB access 3.000 hz age 0 ns          df-passed 878.000 MiB
>     2   addr 127.609 TiB  size 219.418 MiB access 2.000 hz age 0 ns          df-passed 220.000 MiB
>     3   addr 127.609 TiB  size 316.613 MiB access 1.000 hz age 1 s           df-passed 316.000 MiB
[...]
>     memory bw estimate: 6.524 GiB per second  df-passed: 6.527 GiB per second
>     total size: 10.731 GiB  df-passed 10.000 GiB
>     record DAMON intervals: sample 100 ms, aggr 1 s
> 
> 
> $ # damon report again
> $sudo ./damo/damo report access --snapshot_damos_filter allow \
> hugepage_size 2MiB 2MiB

Ditto.

>     heatmap:
>     # min/max temperatures: -1,100,000,000, 2,000, column size: 137.352 MiB
>     intervals: sample 100 ms aggr 1 s (max access hz 10)
>     # damos filters (df): reject none hugepage_size [2.000 MiB, 2.000 MiB]
>     df-pass:
>     # min/max temperatures: -900,000,000, 2,000, column size: 128.031 MiB
>     0   addr 85.373 TiB   size 745.555 MiB access 0 hz   age 11 s          df-passed 0 B
>     1   addr 127.608 TiB  size 579.715 MiB access 2.000 hz age 0 ns          df-passed 580.000 MiB
>     2   addr 127.608 TiB  size 144.930 MiB access 2.000 hz age 0 ns          df-passed 146.000 MiB
>     3   addr 127.608 TiB  size 452.453 MiB access 2.000 hz age 0 ns          df-passed 452.000 MiB
[...]
>     memory bw estimate: 5.297 GiB per second  df-passed: 5.297 GiB per second
>     total size: 10.731 GiB  df-passed 10.000 GiB
>     record DAMON intervals: sample 100 ms, aggr 1 s
> 
> As you can see the total df-passed region is 10GiB and the hot region
> moves as the seq read keeps going
> 
> Revision History
> ================
> Changes from v1 [1]:
> - Follow David's advise to combine *pmd_entry() and *pte_entry() into
>   one. Also remove manually setting walk->action
> - Use vma_normal_page and vma_normal_page_pmd instead of damon_get_folio
>   to remove redundant folio_get and folio_put
> - Follow SJ's advise to only move damon_pa_scheme_has_filter to
>   ops-common
> - Change the command used in cover-letter for more natural illustration
> 
> [1] https://lore.kernel.org/all/cover.1753794408.git.pyyjason@gmail.com/
> 
> Yueyang Pan (2):
>   mm/damon: Move has filter to ops-common
>   mm/damon: Add damos_stat support for vaddr

I also left a few trivial comments to the patches.  So I find no real concerns
but asking for a few trivial cleanups.  Could you please make the changes with
next version, after waiting a day for giving others to chime in for things I
may missed?


Thanks,
SJ

[...]