linux-kernel - Re: [PATCH v3 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: <20250801233942.1614-1-sj@kernel.org>
Date: Fri,  1 Aug 2025 16:39:42 -0700
From: SeongJae Park <sj@...nel.org>
To: 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,
	kernel-team@...a.com
Subject: Re: [PATCH v3 0/2] mm/damon: Add damos_stat support for vaddr

On Fri,  1 Aug 2025 22:59:49 +0000 pyyjason@...il.com wrote:

> From: Yueyang Pan <pyyjason@...il.com>
> 
> Previously damos_stat only supoort paddr. This patch set adds support 
> for damos_stat for vaddr. Also all different types of filters are 
> supported. 

This confused[1] a person.  Technically speaking, DAMOS_STAT action is already
supported on vaddr.  The lack of support is ops level DAMOS filters combined
with DAMOS_STAT action, and this patchset is adding the support.  Could you
please rephrase the subject and the above paragraph?  For example,

    mm/damon/vaddr: support stat-purpose DAMOS filters

    Extend DAMOS_STAT handling of the DAMON operations sets for virtual address
    spaces for ops-level DAMOS filters.

> 
> 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

Seems you missed my comments[2] on your v2 cover letter.  Could you please take
a look?

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

Ditto.  Please check my comments[2] on your v2 cover letter.

[...]
> $ # damon report again
> $sudo ./damo/damo report access --snapshot_damos_filter allow \

Again, please check my comments[2] on your v2 cover letter.

[...]
> 
> Revision History
> ================
> Changes from v2 [2]:
> - Fix some naming and format issues raised by SJ.

Thank you for continuing this great work!

> 
> 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/
> [2] https://lore.kernel.org/all/cover.1753895066.git.pyyjason@gmail.com/
[...]

Thanks,
SJ

[1] https://lore.kernel.org/20250731175827.16060-1-sj@kernel.org
[2] https://lore.kernel.org/20250730200239.60984-1-sj@kernel.org