From: SJ Park <sj@kernel.org>
To: sashiko-bot@kernel.org
Cc: SJ Park <sj@kernel.org>, "Krishna Iyer" <kiyer@crusoe.ai>,
damon@lists.linux.dev
Subject: Re: [PATCH v2 3/3] mm/damon/paddr: support hugetlb folios in access monitoring
Date: Tue, 1 Sep 2026 21:57:40 -0700 [thread overview]
Message-ID: <20260902045742.89356-1-sj@kernel.org> (raw)
In-Reply-To: <20260902031518.8BE6D1F000E9@smtp.kernel.org>
On Wed, 02 Sep 2026 03:15:18 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Non-atomic PTE modification in `damon_hugetlb_mkold()` clobbers hardware-updated dirty bits.
> - [High] DAMOS action appliers suffer from severe O(N) performance overhead on hugetlb pages due to PAGE_SIZE stepping.
> --
>
> commit c04c513b2839f46c6e864892a28fa35c945f1209
> Author: Krishna Iyer <kiyer@crusoe.ai>
>
> mm/damon/paddr: support hugetlb folios in access monitoring
>
> This commit introduces damon_get_monitor_folio() to allow Damon's physical
> address space monitoring to sample hugetlb-backed memory. It updates the
> access monitoring primitives damon_pa_mkold() and damon_pa_young() to use
> this new function. The Damos action appliers continue to use
> damon_get_folio() to skip hugetlb folios.
>
> > The DAMOS action appliers (damon_pa_pageout(),
> > damon_pa_mark_accessed_or_deactivate(), damon_pa_migrate(),
> > damon_pa_stat()) keep using damon_get_folio(): reclaim, LRU
> > manipulation and migration cannot act on hugetlb folios, so their
> > behavior is unchanged.
>
> [Severity: High]
> This is a pre-existing issue, but does this unchanged behavior cause a severe
> performance regression when Damos action appliers encounter hugetlb pages?
>
> When damon_pa_pageout() and damon_pa_stat() (with filters) run on regions
> with hugetlb memory, damon_get_folio() rejects the hugetlb folios. This causes
> the code to fall back to PAGE_SIZE stepping:
>
> mm/damon/paddr.c:damon_pa_pageout() {
> ...
> while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
> folio = damon_get_folio(PHYS_PFN(addr));
> if (damon_pa_invalid_damos_folio(folio, s)) {
> addr += PAGE_SIZE;
> continue;
> }
> ...
> }
>
> For large folios like 1 GiB hugetlb pages, this results in hundreds of thousands
> of redundant atomic refcount operations per page (262,144 times). Does this
> PAGE_SIZE stepping need to be updated to advance by folio_size() to prevent
> massive CPU consumption by Damon threads?
For such cases DAMOS quota feature exists. We could revisit this later, or
whenever a real user reports this as a problem.
>
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index 5c6c3a597fd0b..ed7b7f31291a0 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -37,7 +37,7 @@ static unsigned long damon_pa_core_addr(
> >
> > static void damon_pa_mkold(phys_addr_t paddr)
> > {
> > - struct folio *folio = damon_get_folio(PHYS_PFN(paddr));
> > + struct folio *folio = damon_get_monitor_folio(PHYS_PFN(paddr));
> >
> > if (!folio)
> > return;
>
> [Severity: Critical]
> This is a pre-existing issue, but can this resulting call chain clobber
> hardware-updated dirty bits?
>
> Now that damon_pa_mkold() supports hugetlb folios, it processes them via
> damon_folio_mkold_one(), which reaches damon_hugetlb_mkold(). In this
> function, the PTE is updated non-atomically:
>
> mm/damon/ops-common.c:damon_hugetlb_mkold() {
> ...
> if (pte_young(entry)) {
> referenced = true;
> entry = pte_mkold(entry);
> set_huge_pte_at(mm, addr, pte, entry, psize);
> }
> ...
> }
>
> Any hardware modifications to the PTE (like setting the dirty bit) between
> huge_ptep_get() and set_huge_pte_at() are silently overwritten. Could this
> result in data loss or memory corruption where modified memory is discarded
> without writeback?
I'm separatively working on it.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260902025700.17975-1-kiyer@crusoe.ai?part=3
Thanks,
SJ
next prev parent reply other threads:[~2026-09-02 4:57 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-02 2:56 [PATCH v2 0/3] mm/damon: support access monitoring of hugetlb-backed memory Krishna Iyer
2026-09-02 2:56 ` [PATCH v2 1/3] mm/damon: move damon_hugetlb_mkold() from vaddr to ops-common Krishna Iyer
2026-09-02 3:14 ` sashiko-bot
2026-09-02 4:51 ` SJ Park
2026-09-02 2:56 ` [PATCH v2 2/3] mm/damon/ops-common: handle hugetlb folios in folio mkold/young rmap walkers Krishna Iyer
2026-09-02 3:12 ` sashiko-bot
2026-09-02 4:53 ` SJ Park
2026-09-02 5:05 ` SJ Park
2026-09-02 2:57 ` [PATCH v2 3/3] mm/damon/paddr: support hugetlb folios in access monitoring Krishna Iyer
2026-09-02 3:15 ` sashiko-bot
2026-09-02 4:57 ` SJ Park [this message]
2026-09-02 5:17 ` SJ Park
2026-09-02 5:29 ` [PATCH v2 0/3] mm/damon: support access monitoring of hugetlb-backed memory SJ 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=20260902045742.89356-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=damon@lists.linux.dev \
--cc=kiyer@crusoe.ai \
--cc=sashiko-bot@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 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.