linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: Yueyang Pan <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
Subject: Re: [PATCH v1 0/2] mm/damon: Add damos_stat support for vaddr
Date: Tue, 29 Jul 2025 11:34:59 -0700	[thread overview]
Message-ID: <20250729183459.56512-1-sj@kernel.org> (raw)
In-Reply-To: <cover.1753794408.git.pyyjason@gmail.com>

Hello Pan (Or, should I call you Yueyang or Jason?  Please let me know your
preferrence if you have),

On Tue, 29 Jul 2025 06:53:28 -0700 Yueyang Pan <pyyjason@gmail.com> wrote:

> From: PanJason <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. 
> 
> 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 record "./my_test/test" --monitoring_intervals 100000\
> 1000000 2000000

You can use 'start' instead of 'record' for this test purpose.

--monitoring_intervals receive more human-friendly format, so you can do
'--monitoring_intervals 100ms 1s 2s'.

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

The --snapshot_damos_filter option means you want to make folios that not
having size 2 MiB not passed by the filter.  You can ask same thing in more
intuitive way, like below.

    --snapshot_damos_filter allow hugepage_size 2MiB 2MiB

> heatmap:
> # min/max temperatures: -900,000,000, 100,002,000, column size: 136.564 MiB

checkpatch.pl gives me below warning:

    WARNING: Commit log lines starting with '#' are dropped by git as comments

I'd suggest adding four spaces prefix to quoted command outputs like this.

> 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: -663,075,528, 100,002,000, column size: 128.037 MiB
> 0   addr 86.082 TiB   size 682.039 MiB access 0 hz   age 9 s           df-passed 0 B
> 1   addr 127.225 TiB  size 466.039 MiB access 1.000 hz age 0 ns          df-passed 468.000 MiB
[...]
> memory bw estimate: 3.615 GiB per second  df-passed: 3.615 GiB per second
> total size: 10.669 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 reject none \
> hugepage_size 2MiB 2MiB
> heatmap:
> # min/max temperatures: -1,100,000,000, 300,001,000, column size: 136.564 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: -800,000,000, 300,001,000, column size: 128.037 MiB
> 0   addr 86.082 TiB   size 682.039 MiB access 0 hz   age 11 s          df-passed 0 B
> 1   addr 127.225 TiB  size 10.355 MiB  access 1.000 hz age 0 ns          df-passed 12.000 MiB
> 2   addr 127.225 TiB  size 93.207 MiB  access 1.000 hz age 0 ns          df-passed 92.000 MiB
> 3   addr 127.225 TiB  size 414.262 MiB access 1.000 hz age 0 ns          df-passed 414.000 MiB
> 4   addr 127.225 TiB  size 706.695 MiB access 1.000 hz age 3 s           df-passed 708.000 MiB
> 5   addr 127.226 TiB  size 78.523 MiB  access 1.000 hz age 3 s           df-passed 78.000 MiB
[...]
> total size: 10.669 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
> 
> PanJason (2):
>   mm/damon: Move invalid folio and has filter to ops-common
>   mm/damon: Add damos_stat support for vaddr

The changes look good overall, though I left a few comments for formatting and
unnecessary folio references that inherited from my original sin.  Looking
forward to more discussions and next version of this great patch series!


Thanks,
SJ

[...]


      parent reply	other threads:[~2025-07-29 18:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-29 13:53 [PATCH v1 0/2] mm/damon: Add damos_stat support for vaddr Yueyang Pan
2025-07-29 13:53 ` [PATCH v1 1/2] mm/damon: Move invalid folio and has filter to ops-common Yueyang Pan
2025-07-29 18:20   ` SeongJae Park
2025-07-29 13:53 ` [PATCH v1 2/2] mm/damon: Add damos_stat support for vaddr Yueyang Pan
2025-07-29 14:11   ` David Hildenbrand
2025-07-29 17:46     ` SeongJae Park
2025-07-30 22:24     ` Yueyang Pan
2025-07-29 18:15   ` SeongJae Park
2025-07-29 18:21   ` SeongJae Park
2025-07-29 18:34 ` 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=20250729183459.56512-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --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).