From: SeongJae Park <sj@kernel.org>
To: Gutierrez Asier <gutierrez.asier@huawei-partners.com>
Cc: SeongJae Park <sj@kernel.org>,
artem.kuzin@huawei.com, stepanov.anatoly@huawei.com,
wangkefeng.wang@huawei.com, yanquanmin1@huawei.com,
zuoze1@huawei.com, damon@lists.linux.dev,
akpm@linux-foundation.org, ljs@kernel.org,
Liam.Howlett@oracle.com, vbabka@kernel.org, rppt@kernel.org,
surenb@google.com, mhocko@suse.com, corbet@lwn.net,
skhan@linuxfoundation.org, linux-doc@vger.kernel.org,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 1/1] This patch set introces a new action: DAMOS_COLLAPSE.
Date: Tue, 24 Mar 2026 07:12:57 -0700 [thread overview]
Message-ID: <20260324141257.91322-1-sj@kernel.org> (raw)
In-Reply-To: <48580762-eec3-49b6-b17a-59fa486bebec@huawei-partners.com>
On Tue, 24 Mar 2026 16:57:22 +0300 Gutierrez Asier <gutierrez.asier@huawei-partners.com> wrote:
>
>
> On 3/24/2026 3:39 AM, SeongJae Park wrote:
> > Hello Asier,
> >
> > On Mon, 23 Mar 2026 14:56:45 +0000 <gutierrez.asier@huawei-partners.com> wrote:
> >
> >> From: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> >>
> >> For DAMOS_HUGEPAGE and DAMOS_NOHUGEPAGE to work, khugepaged should be
> >> working, since it relies on hugepage_madvise to add a new slot. This
> >> slot should be picked up by khugepaged and eventually collapse (or
> >> not, if we are using DAMOS_NOHUGEPAGE) the pages. If THP is not
> >> enabled, khugepaged will not be working, and therefore no collapse
> >> will happen.
> >>
> >> DAMOS_COLLAPSE eventually calls madvise_collapse, which will collapse
> >> the address range synchronously.
> >>
> >> This new action may be required to support autotuning with hugepage as
> >> a goal[1].
> >>
> >> [1]: https://lore.kernel.org/damon/20260313000816.79933-1-sj@kernel.org/
> >>
> >> ---------
> >> Benchmarks:
> >>
> >> T n: THP never
> >> T m: THP madvise
> >> D h: DAMON action hugepage
> >> D c: DAMON action collapse
> >>
> >> +------------------+----------+----------+----------+
> >> | | T n, D h | T m, D h | T n, D c |
> >> +------------------+----------+----------+----------+
> >> | Total memory use | 2.07 | 2.09 | 2.07 |
> >> | Huge pages | 0 | 1.3 | 1.25 |
> >> +------------------+----------+----------+----------+
> >
> > Thank you for sharing the benchmark results! But, I'm having a hard time to
> > understand what this really means. Could you please further clarify the setup
> > of the benchmarks and interpretation of the results?
> I will fix the cover in the next version, which I will submit soon.
>
> I tested the patch in a physical server with MariaDB 10.5. I run
> sysbench to load the server.
>
> I check 3 scenarios:
> - DAMON action hugepage for the database task, THP as never
> - DAMON action hugepage, THP madvise
> - DAMON action collapse, THP never
>
> I compared the memory consumption, both in overall in the server and
> anonymous huge page consumption. The results are in the table
>
> T n: THP never
> T m: THP madvise
> D h: DAMON action hugepage
> D c: DAMON action collapse
Thank you for sharing the details of the setup, Asier.
>
> +------------------+----------+----------+----------+
> | | T n, D h | T m, D h | T n, D c |
> +------------------+----------+----------+----------+
> | Total memory use | 2.07 | 2.09 | 2.07 |
> | Huge pages | 0 | 1.3 | 1.25 |
> +------------------+----------+----------+----------+
Could you please further share interpretation of the results? What these
results mean? Does it show some benefit of DAMOS_COLLAPSE?
Also, how is the performance? Does it show some difference?
> >
> >>
> >> Changes
> >> ---------
> >> v1-v2:
> >> Added benchmarks
> >> Added damos_filter_type documentation for new action to fix kernel-doc
> >
> > Please add Changelog on the commentary section [1]. Also, please consider
> > adding links to previous versions.
> >
> >>
> >> Signed-off-by: Asier Gutierrez <gutierrez.asier@huawei-partners.com>
> >> ---
> >> Documentation/mm/damon/design.rst | 4 ++++
> >> include/linux/damon.h | 2 ++
> >> mm/damon/sysfs-schemes.c | 4 ++++
> >> mm/damon/vaddr.c | 3 +++
> >> tools/testing/selftests/damon/sysfs.py | 11 ++++++-----
> >> 5 files changed, 19 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> >> index 838b14d22519..405142641e55 100644
> >> --- a/Documentation/mm/damon/design.rst
> >> +++ b/Documentation/mm/damon/design.rst
> >> @@ -467,6 +467,10 @@ that supports each action are as below.
> >> Supported by ``vaddr`` and ``fvaddr`` operations set. When
> >> TRANSPARENT_HUGEPAGE is disabled, the application of the action will just
> >> fail.
> >> + - ``collapse``: Call ``madvise()`` for the region with ``MADV_COLLAPSE``.
> >> + Supported by ``vaddr`` and ``fvaddr`` operations set. When
> >> + TRANSPARENT_HUGEPAGE is disabled, the application of the action will just
> >> + fail.
> >> - ``lru_prio``: Prioritize the region on its LRU lists.
> >> Supported by ``paddr`` operations set.
> >> - ``lru_deprio``: Deprioritize the region on its LRU lists.
> >> diff --git a/include/linux/damon.h b/include/linux/damon.h
> >> index d9a3babbafc1..6941113968ec 100644
> >> --- a/include/linux/damon.h
> >> +++ b/include/linux/damon.h
> >> @@ -121,6 +121,7 @@ struct damon_target {
> >> * @DAMOS_PAGEOUT: Reclaim the region.
> >> * @DAMOS_HUGEPAGE: Call ``madvise()`` for the region with MADV_HUGEPAGE.
> >> * @DAMOS_NOHUGEPAGE: Call ``madvise()`` for the region with MADV_NOHUGEPAGE.
> >> + * @DAMOS_COLLAPSE: Call ``madvise()`` for the region with MADV_COLLAPSE.
> >> * @DAMOS_LRU_PRIO: Prioritize the region on its LRU lists.
> >> * @DAMOS_LRU_DEPRIO: Deprioritize the region on its LRU lists.
> >> * @DAMOS_MIGRATE_HOT: Migrate the regions prioritizing warmer regions.
> >> @@ -140,6 +141,7 @@ enum damos_action {
> >> DAMOS_PAGEOUT,
> >> DAMOS_HUGEPAGE,
> >> DAMOS_NOHUGEPAGE,
> >> + DAMOS_COLLAPSE,
> >> DAMOS_LRU_PRIO,
> >> DAMOS_LRU_DEPRIO,
> >> DAMOS_MIGRATE_HOT,
> >> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> >> index 5186966dafb3..aa08a8f885fb 100644
> >> --- a/mm/damon/sysfs-schemes.c
> >> +++ b/mm/damon/sysfs-schemes.c
> >> @@ -2041,6 +2041,10 @@ static struct damos_sysfs_action_name damos_sysfs_action_names[] = {
> >> .action = DAMOS_NOHUGEPAGE,
> >> .name = "nohugepage",
> >> },
> >> + {
> >> + .action = DAMOS_COLLAPSE,
> >> + .name = "collapse",
> >> + },
> >> {
> >> .action = DAMOS_LRU_PRIO,
> >> .name = "lru_prio",
> >> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> >> index b069dbc7e3d2..dd5f2d7027ac 100644
> >> --- a/mm/damon/vaddr.c
> >> +++ b/mm/damon/vaddr.c
> >> @@ -903,6 +903,9 @@ static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
> >> case DAMOS_NOHUGEPAGE:
> >> madv_action = MADV_NOHUGEPAGE;
> >> break;
> >> + case DAMOS_COLLAPSE:
> >> + madv_action = MADV_COLLAPSE;
> >> + break;
> >> case DAMOS_MIGRATE_HOT:
> >> case DAMOS_MIGRATE_COLD:
> >> return damos_va_migrate(t, r, scheme, sz_filter_passed);
> >> diff --git a/tools/testing/selftests/damon/sysfs.py b/tools/testing/selftests/damon/sysfs.py
> >> index 3aa5c91548a5..c6476e63f4fb 100755
> >> --- a/tools/testing/selftests/damon/sysfs.py
> >> +++ b/tools/testing/selftests/damon/sysfs.py
> >> @@ -123,11 +123,12 @@ def assert_scheme_committed(scheme, dump):
> >> 'pageout': 2,
> >> 'hugepage': 3,
> >> 'nohugeapge': 4,
> >> - 'lru_prio': 5,
> >> - 'lru_deprio': 6,
> >> - 'migrate_hot': 7,
> >> - 'migrate_cold': 8,
> >> - 'stat': 9,
> >> + 'collapse': 5
> >
> > Comman is missed?
> >
> >> + 'lru_prio': 6,
> >> + 'lru_deprio': 7,
> >> + 'migrate_hot': 8,
> >> + 'migrate_cold': 9,
> >> + 'stat': 10,
> >> }
> >> assert_true(dump['action'] == action_val[scheme.action], 'action', dump)
> >> assert_true(dump['apply_interval_us'] == scheme. apply_interval_us,
> >> --
> >> 2.43.0
> >
> > Other than the selftest part, code looks good. Please consider dropping RFC
> > tag from the next spin. Clarifying more details about the test would be
> > helpful, though.
> >
> > [1] https://docs.kernel.org/process/submitting-patches.html#commentary
> >
> >
> > Thanks,
> > SJ
> >
>
> SJ, should I remove the RFC tag for the next version?
If you feel comfortable with it, please do so.
Thanks,
SJ
[...]
next prev parent reply other threads:[~2026-03-24 14:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-23 14:56 [RFC PATCH v1 1/1] This patch set introces a new action: DAMOS_COLLAPSE gutierrez.asier
2026-03-23 14:56 ` [RFC PATCH v2 1/1] mm/damon: support MADV_COLLAPSE via DAMOS_COLLAPSE scheme action gutierrez.asier
2026-03-24 0:29 ` (sashiko review) " SeongJae Park
2026-03-24 0:41 ` SeongJae Park
2026-03-24 13:49 ` Gutierrez Asier
2026-03-24 0:39 ` [RFC PATCH v1 1/1] This patch set introces a new action: DAMOS_COLLAPSE SeongJae Park
2026-03-24 13:57 ` Gutierrez Asier
2026-03-24 14:12 ` SeongJae Park [this message]
2026-03-24 15:01 ` Gutierrez Asier
2026-03-25 1:29 ` 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=20260324141257.91322-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=Liam.Howlett@oracle.com \
--cc=akpm@linux-foundation.org \
--cc=artem.kuzin@huawei.com \
--cc=corbet@lwn.net \
--cc=damon@lists.linux.dev \
--cc=gutierrez.asier@huawei-partners.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=ljs@kernel.org \
--cc=mhocko@suse.com \
--cc=rppt@kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=stepanov.anatoly@huawei.com \
--cc=surenb@google.com \
--cc=vbabka@kernel.org \
--cc=wangkefeng.wang@huawei.com \
--cc=yanquanmin1@huawei.com \
--cc=zuoze1@huawei.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