linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: pyyjason@gmail.com
Cc: SeongJae Park <sj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Usama Arif <usamaarif642@gmail.com>,
	damon@lists.linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, kernel-team@meta.com
Subject: Re: [PATCH v3 0/2] mm/damon: Add damos_stat support for vaddr
Date: Fri,  1 Aug 2025 16:39:42 -0700	[thread overview]
Message-ID: <20250801233942.1614-1-sj@kernel.org> (raw)
In-Reply-To: <cover.1754088635.git.pyyjason@gmail.com>

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

> From: Yueyang Pan <pyyjason@gmail.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


      parent reply	other threads:[~2025-08-01 23:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-01 22:59 [PATCH v3 0/2] mm/damon: Add damos_stat support for vaddr pyyjason
2025-08-01 22:59 ` [PATCH v3 1/2] mm/damon: Move has filter to ops-common pyyjason
2025-08-01 23:30   ` SeongJae Park
2025-08-01 22:59 ` [PATCH v3 2/2] mm/damon: Add damos_stat support for vaddr pyyjason
2025-08-01 23:39   ` SeongJae Park
2025-08-01 23:39 ` SeongJae Park [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250801233942.1614-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=pyyjason@gmail.com \
    --cc=usamaarif642@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).