All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: gutierrez.asier@huawei-partners.com
Cc: damon@lists.linux.dev
Subject: Re: [PATCH v1 2/3] mm/damon: introduce DAMON_HUGEPAGE for hot region hugepage collapsing
Date: Tue, 16 Jun 2026 15:21:39 +0000	[thread overview]
Message-ID: <20260616152139.9648D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260616150316.580819-3-gutierrez.asier@huawei-partners.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The `DAMOS_QUOTA_USER_INPUT` goal tuner feedback mechanism is statically stuck because `current_value` is never dynamically updated.
- [Medium] The scheme applies the DAMOS_COLLAPSE action to all regions including completely cold ones, contradicting the patch description.
- [Low] The module cannot be compiled due to a naming mismatch between the Kconfig symbol and the Makefile variable.
--

commit 7e15caa860a55beb8439fc6bb5f299c5c42fb138
Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>

mm/damon: introduce DAMON_HUGEPAGE for hot region hugepage collapsing

This patch introduces a new DAMON module (SAMPLE_DAMON_HPAGE)
which collapses hot regions into huge pages. The module monitors
a specific task using DAMON-based sampling and applies collapsing
based on memory usage.

> diff --git a/samples/damon/Makefile b/samples/damon/Makefile
> index 72f68cbf422aa..a348dc74ddcb1 100644
> --- a/samples/damon/Makefile
> +++ b/samples/damon/Makefile
> @@ -3,3 +3,4 @@
>  obj-$(CONFIG_SAMPLE_DAMON_WSSE) += wsse.o
>  obj-$(CONFIG_SAMPLE_DAMON_PRCL) += prcl.o
>  obj-$(CONFIG_SAMPLE_DAMON_MTIER) += mtier.o
> +obj-$(CONFIG_SAMPLE_DAMON_HUGEPAGE) += hpage.o

[Severity: Low]
Does this prevent the module from building? 

The Kconfig file defines the configuration option as SAMPLE_DAMON_HPAGE, 
but the Makefile evaluates CONFIG_SAMPLE_DAMON_HUGEPAGE. This mismatch 
means the build system might silently skip compiling hpage.o.

> diff --git a/samples/damon/hpage.c b/samples/damon/hpage.c
> new file mode 100644
> index 0000000000000..ebbe5e1be1a1d
> --- /dev/null
> +++ b/samples/damon/hpage.c

[ ... ]

> +	scheme = damon_new_scheme(&(struct damos_access_pattern) {
> +				.min_sz_region = HPAGE_PMD_SIZE,
> +				.max_sz_region = ULONG_MAX,
> +				.min_nr_accesses = 0,
> +				.max_nr_accesses = UINT_MAX,

[Severity: Medium]
Will this apply the collapse action to completely cold regions? 

The commit message states this module collapses "hot regions", but setting 
min_nr_accesses to 0 instructs DAMON to apply the action to all regions 
that meet the age threshold, including those with no accesses at all. 

Should this be set to a value strictly greater than 0 to only target 
heavily accessed regions?

[ ... ]

> +	if (quota_autotune_feedback) {
> +		goal = damos_new_quota_goal(DAMOS_QUOTA_USER_INPUT, 10000);
> +		if (!goal) {
> +			damon_destroy_ctx(ctx);
> +			return -ENOMEM;
> +		}
> +		goal->current_value = quota_autotune_feedback;
> +		damos_add_quota_goal(&scheme->quota, goal);
> +	}

[Severity: Medium]
Is there a missing mechanism to dynamically update this feedback value?

Since the module parameter is assigned to current_value exactly once at 
startup, without a dynamic update path while running, the auto-tuner will 
receive a constant error value. 

Won't this continuously push the quota aggressiveness in one direction 
until it hits the limits, rather than forming a proper feedback loop?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616150316.580819-1-gutierrez.asier@huawei-partners.com?part=2

  reply	other threads:[~2026-06-16 15:21 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-16 15:03 [PATCH v1 0/3] mm/damon: Introduce a huge page collapsing mechanism using auto tuning gutierrez.asier
2026-06-16 15:03 ` [PATCH v1 1/3] mm/damon: Introduce DAMOS_QUOTA_HUGEPAGE " gutierrez.asier
2026-06-16 15:20   ` sashiko-bot
2026-06-16 19:33     ` Gutierrez Asier
2026-06-17  3:31   ` SeongJae Park
2026-06-16 15:03 ` [PATCH v1 2/3] mm/damon: introduce DAMON_HUGEPAGE for hot region hugepage collapsing gutierrez.asier
2026-06-16 15:21   ` sashiko-bot [this message]
2026-06-16 19:27     ` Gutierrez Asier
2026-06-17  4:09       ` SeongJae Park
2026-06-17  4:04   ` SeongJae Park
2026-06-16 15:03 ` [PATCH v1 3/3] mm/damon/sysfs: support hugepage_mem_bp quota goal metric gutierrez.asier
2026-06-16 15:21   ` sashiko-bot
2026-06-16 19:35     ` Gutierrez Asier
2026-06-17  4:15       ` SeongJae Park
2026-06-17  4:16   ` SeongJae Park
2026-06-17  1:44 ` [PATCH v1 0/3] mm/damon: Introduce a huge page collapsing mechanism using auto tuning 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=20260616152139.9648D1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=gutierrez.asier@huawei-partners.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.