From: SeongJae Park <sj@kernel.org>
To: Akinobu Mita <akinobu.mita@gmail.com>
Cc: SeongJae Park <sj@kernel.org>,
damon@lists.linux.dev, linux-perf-users@vger.kernel.org
Subject: Re: [RFC PATCH v3 0/4] mm/damon: introduce perf event based access check
Date: Fri, 24 Apr 2026 16:31:34 -0700 [thread overview]
Message-ID: <20260424233136.6716-1-sj@kernel.org> (raw)
In-Reply-To: <CAC5umyiikHr4doYKW5Gy4uv6M0_HbBZUSd9kM+L+kCrKYKEuXw@mail.gmail.com>
On Fri, 24 Apr 2026 12:27:07 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:
> Hello SeongJae,
>
> 2026年4月23日(木) 13:34 SeongJae Park <sj@kernel.org>:
> >
> > Hello Akinobu,
> >
> > On Thu, 23 Apr 2026 09:42:06 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:
> >
> > > DAMON currently only provides PTE accessed-bit based access check, this
> > > patch series adds a new perf event based access check.
> > >
> > > Since perf event-based access checks do not require modifying the PTE
> > > accessed-bit for pages representing each damon region, it reduces the
> > > overhead of monitoring at a fixed granularity of the page size.
> >
> > As I also commented to the previous version, the high level idea makes sense to
> > me. I think this can be useful.
> >
> > Also as I commented to the previous version, I understand the page level
> > monitoring overhead reduction is the main purpose of this patch, and it makes
> > sense to me. Nonetheless, I like this mainly because the perf event could
> > provide more detailed information including from which CPU/thread the access is
> > made, and whether the access is for read/write, to my understanding. Please
> > let me know if my understanding is wrong.
>
> That's correct.
> Once the integration into your new monitoring infrastructure is
> complete, those things will become possible.
Thank you for confirming!
>
> > >
> > > Here is a method and its results for comparing access checks using
> > > existing access bits and new perf events.
> > [...]
> > > Using accessed bit, prepare_access_checks takes 7 seconds and
> > > check_accesses takes 5 seconds.
> > [...]
> > > Using perf event, prepare_access_checks takes 0.01 seconds and
> > > check_accesses takes 2.6 seconds.
> >
> > Thank you so much for sharing these great test results with the detailed setup
> > description!
> >
> > > ```
> > > $ sudo $HOME/damo/damo stop
> > > $ sudo $HOME/damo/damo start --ops vaddr \
> > > --perf_event 5000 0 0x4 0x1cd 0x1f 0 \
> > > --monitoring_intervals 5s 60s 300s \
> > > --monitoring_nr_regions_range 1000000000 1000000000 \
> > > --target_pid $(cat /tmp/memcached.pid)
> > [...]
> > > Note: damo's --perf_event option
> > >
> > > Using these features also requires modifications to damo, but these
> > > are not included in this patch series and are currently under
> > > development in the following branch:
> > >
> > > * https://github.com/mita/damo/tree/damo-perf-for-v3.2.2
> >
> > Thank you for sharing this, too!
> >
> > >
> > > The option newly added to damo for perf event-based access check has the
> > > following format:
> > >
> > > `--perf_event <sample freq> <sample phys addr> <type> <config> <config1> <config2>`
> >
> > I think we may need to discuss more for final interface. But I think that
> > could be done later, after the kerenl ABI is fixed.
>
> My current patch introduces a set of kernel ABIs (.../perf_events/<P>/*),
> and if I want to control another field in the perf_event_attr,
> I would have to add a new ABI. This would be a significant
> maintenance cost.
>
> Instead, I would like to provide a single bin file where the
> perf_event_attr I want to set is written as binary.
I'd prefer having single file per parameter, as sysfs is designed for. It
allows keeping the input format simple and therefore easy to maintain and
extend. Of course, we should carefult at adding new parameters, though.
>
> > >
> > > - `sample freq`: A higher frequency improves access accuracy, but also
> > > increases overhead.
> > > - `sample phys addr`: specify 0 for vaddr and 1 for paddr.
> > > - The remaining type, config, config1, and config2 settings can be found
> > > as follows:
> >
> > The values look bit human-unfriendly. I wonder if we could use more
> > human-friendly values, e.g., 'cpu' for type,
> > 'mem_trans_retired.load_latency_gt_1024' for config, etc. Not necessarily we
> > need to fix this right now. Let's keep discussing.
>
> I'd like to do it that way. These processes can be handled better
> from user space than from within the kernel, so I'd like to achieve
> this from damo with the help of the perf tool and library.
Does existing perf event ABI also take that approach? If so, I think that is
fine.
>
> > >
> > > Run `perf mem record -vv` to obtain the log, and then find the part in
> > > the log where `perf_event_open` was executed successfully.
> > > ```
> > > $ sudo perf mem record -vv echo > /tmp/perf-mem-record.log 2>&1
> > > $ cat /tmp/perf-mem-record.log
> > > ...
> > > perf_event_attr:
> > > type 4 (cpu)
> > > size 144
> > > config 0x1cd (mem_trans_retired.load_latency_gt_1024)
> > [...]
> > > ...
> > > ```
> > >
> > > The values for `type` and `config` are taken from this output.
> > > Use the values of `config1` and `config2` if they are included in the log;
> > > otherwise, set them to 0.
> >
> > The above information will also be very useful for early testers of this patch
> > series including myself. Thank you for sharing.
> >
> > >
> > > Any feedback or advice on the patch set would be greatly appreciated.
> >
> > Again, I like the high level concept of this patch series. I definitely want
> > this to be merged. Nonetheless, not exactly as it is right now. I think we
> > should spend enough time for the user interface design. I hope the final
> > interface is long-term maintainable and easy to extend.
> >
> > >
> > > * RFC v3
> > > - drop "reintroduce damon_operations->cleanup()" as no longer needed
> > > - drop "allow user to set min size of region" (will be sent separately)
> > > - add tgid to struct damon_access_report and use it instead of tid
> > > - set perf_event_attr.precise_ip to 2
> > > - prepare for the transition to report-based monitoring
> > > - switch to use access_reported in struct damon_region
> > > - switch to use sample/primitives/perf_events sysfs directory
> >
> > Thank you for doing this revisioning.
> >
> > For the user interface and also internal interface, I suggested to consider
> > reusing those for page fault events based per-CPUs/threads/reads/writes
> > monitoring DAMON extension patch series in our previous discussion. I think
> > this version is following the right direction much more than the previous
> > version.
> >
> > But, nowadays I'm thinking about a new user interface for the
> > per-CPUs/threads/reads/writes monitoring that can easily also extended for perf
> > event based monitoring and for not only access monitoring but general and
> > multiple data attributes monitoring.
> >
> > It is still under the development and I'm planning to share a very early RFC
> > version of that by LSFMMBPF. I will try to add my idea about how this perf
> > event based monitoring could also implemented by reusing the interface on the
> > early RFC version.
> >
> > Could we wait until the early RFC version is published, review my idea about
> > how this perf event based monitoring could reuse it, and continue discussions
> > about the next steps of this patch series?
>
> That sounds good. I'd definitely like to check it out.
Thank you for accepting my humble suggestion. And sorry for making you be
delayed. Hopefully this interface extension will soon be published as it is
one of the topics for DAMON session [1] at LSFMMBPF.
>
> > [1] https://lore.kernel.org/20251208062943.68824-1-sj@kernel.org/
[1] https://docs.google.com/spreadsheets/d/1mGEdDrWskp7Ua91jGXzquQGinorcD58DAVXhOiRp2Gg/edit?gid=1852749899#gid=1852749899
Thanks,
SJ
[...]
next prev parent reply other threads:[~2026-04-24 23:31 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-23 0:42 [RFC PATCH v3 0/4] mm/damon: introduce perf event based access check Akinobu Mita
2026-04-23 0:42 ` [RFC PATCH v3 1/4] mm/damon/core: add code borrowed from report-based monitoring work Akinobu Mita
2026-04-23 1:21 ` sashiko-bot
2026-04-23 0:42 ` [RFC PATCH v3 2/4] mm/damon/core: add common code for perf event based access check Akinobu Mita
2026-04-23 1:58 ` sashiko-bot
2026-04-23 0:42 ` [RFC PATCH v3 3/4] mm/damon/vaddr: support " Akinobu Mita
2026-04-23 2:48 ` sashiko-bot
2026-04-23 0:42 ` [RFC PATCH v3 4/4] mm/damon/paddr: " Akinobu Mita
2026-04-23 3:22 ` sashiko-bot
2026-04-23 4:34 ` [RFC PATCH v3 0/4] mm/damon: introduce " SeongJae Park
2026-04-24 3:27 ` Akinobu Mita
2026-04-24 23:31 ` SeongJae Park [this message]
2026-04-25 12:33 ` Akinobu Mita
2026-04-25 15:33 ` SeongJae Park
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=20260424233136.6716-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=akinobu.mita@gmail.com \
--cc=damon@lists.linux.dev \
--cc=linux-perf-users@vger.kernel.org \
/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