All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.