From: SeongJae Park <sj@kernel.org>
To: Liew Rui Yan <aethernet65535@gmail.com>
Cc: SeongJae Park <sj@kernel.org>, damon@lists.linux.dev, linux-mm@kvack.org
Subject: Re: [Question] mm/damon: min_nr_regions constraint and inconsistent error handling
Date: Wed, 18 Mar 2026 18:23:36 -0700 [thread overview]
Message-ID: <20260319012337.96726-1-sj@kernel.org> (raw)
In-Reply-To: <20260318153731.97470-1-aethernet65535@gmail.com>
Hi Liew,
On Wed, 18 Mar 2026 23:37:31 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> Hi SeongJae,
>
> TL;DR
> =====
> 1. Why does DAMON require min_nr_regions >= 3? Is there a technical
> rationale?
> 2. Could we improve error reporting when sysfs parameter validation
> fails? Or document it?
> 3. [Bug?] DAMON_LRU_SORT doesn't disable itself when commit_inputs fails,
> contrary to documentation.
Thank you for nice questions, I will reply below in line.
>
> Details below. Happy to help with patches if helpful.
>
> Details
> =======
>
> 1. Error feedback for min_nr_regions validation
> -----------------------------------------------
>
> While configuring DAMON_LRU_SORT via sysfs, I noticed that setting
> 'min_nr_regions' to a value < 3 (e.g., 1) succeeds at write time, but
> the error only appears later when writing 'Y' to the 'enabled' file,
> which returns -EINVAL.
>
> Since multiple parameters (intervals, watermarks, etc.) are often
> configured together, a generic -EINVAL at enablement makes it difficult
> to identify which specific parameter violated constraints.
>
> Code reference: mm/damon/core.c:damon_set_attrs()
>
> if (attrs->min_nr_regions < 3)
> return -EINVAL;
>
> Question:
> 1. What is the design rationale for requiring min_nr_regions >= 3? (eg.,
> algorithmic requirements, sampling accuracy, etc.)
The initial version of DAMON was supporting only 'vaddr' operation set. And
'vaddr' is designed to have at least three regions, for the two large unmapped
areas on normal virtual address spaces. Hence we enforced min_nr_regions to be
three or larger.
There is no real reason to keep it for 'fvaddr' and 'paddr'. Maybe we can lift
the restriction for 'fvaddr' and 'paddr' operation sets. But, we found no real
reason to lift it, either. As a result, the restriction is still there. If
you find a reason to lift the restriction, please feel free to let me know, or
send patches for that.
The VMA-based target address range construction desgin doc [1] should explain
more details about the two large unmapped areas. Please feel free to ask more
clarifications if it doesn't give you enough details.
> 2. Would it be acceptable to improve user feedback? For example:
> - Document this lower bound explicitly in
> Documentation/admin-guide/mm/damon/lru_sort.rst
Yes, I think this is a nice idea. Maybe adding a comment on damon_set_attrs(),
or additional sentences to 'Monitoring' section of design.rst [2] would be
appropriate.
> - Add pr_warn()/pr_debug() in damon_set_attrs() to log which
> parameter failed validation and why
I'd like to be cautious about adding logging, as it might be a spam. I'd
prefer making it documented well. Then, users could find what is wrong.
>
> I believe clearer errors or document would make DAMON tuning more
> intuitive for sysadmins and developers.
I agree, the documentation should be helpful.
>
> ---
>
> 2. Potential inconsistency in DAMON_LRU_SORT error handling
> -----------------------------------------------------------
>
> The documentation [1] states:
> > "If invalid parameters are found while the re-reading, DAMON_LRU_SORT
> will be disabled."
>
> However, testing on v7.0-rc4 shows that when commit_inputs fails (e.g.,
> due to min_nr_regions < 3):
> - The 'enabled' parameter remains 'Y'
> - The kdamond thread continues running
>
> This behavior may confuse users who expect the module to safely disable
> itself upon configuration errors. Should commit_inputs failures trigger
> an explicit disable (enabled=N)?
Nice finding! In short, it is an intended behavior, but the document is
outdated.
In the past, the documentation was correct. When a wrong input is committed,
DAMON_LRU_SORT was stopped. That's because it was using damon_callback, which
is now removed [3]. The callback was executed inside DAMON main loop, and when
the callback returns an error, the loop was broken. As a result, DAMON
execution was stopped. DAMON_LRU_SORT was setting the parameter inside the
callback, and was returning an error when the parameter is invalid. Hence the
behavior was documented as you noticed.
Commit a30969436428 ("mm/damon/lru_sort: use damon_commit_ctx()") changed
the behavior. It changed DAMON_LRU_SORT to use damon_call() instead of
damon_callback. Because damon_call()'s callback function failure doesn't stop
DAMON, the current behavior is made.
Still, committing wrong DAMON_LRU_SORT parameters returns an error to the user.
Hence user can notify it was failed. The invalid parameters are not really be
committed to DAMON, so DAMON continues running with valid parameters. So this
is technically speaking a visible user behavior change. But we believed it is
ok because we were unable to expect users who depend on the old behavior. I
think I discussed this somewhere, but I cannot find the link right now...
>
> Reproduction steps:
> -------------------
> VM: virtme-ng 1.40 (x86_64)
> Kernel: v7.0-rc4
>
> cd /sys/module/damon_lru_sort/parameters
> cat enabled
> N
>
> # Initial setup - works fine
> echo 65535 > max_nr_regions
> echo 3 > min_nr_regions
> echo Y > enabled
> cat kdamond_pid
> 91
>
> # Observed behavior:
> echo 1 > min_nr_regions
> echo Y > commit_inputs
I believe the above command should returned an error. If not, please let me
know.
> cat kdamond_pid
> 91
> cat enabled
> Y
>
> # [kdamond.0] still alive
> ps aux | rg "kdamond"
> root 91 0.0 0.0 0 0 ? I 22:03 0:00 [kdamond.0]
>
> If this is confirmed as unintended behavior, I'd be glad to help
> investigate and submit a fix.
So, this is an intended behavior. But the document is outdated. It would be
nice if you could send a patch for fixing the document.
Also, if you know users who depend on the old behavior and therefore hoping the
old behavior back, please let me know.
>
> ---
>
> Thanks for your time reviewing this!
Thank you for sharing your findings and questions!
>
> Best regards,
> Rui Yan
>
> [1] https://docs.kernel.org/admin-guide/mm/damon/lru_sort.html
>
> [...]
[1] https://docs.kernel.org/mm/damon/design.html#vma-based-target-address-range-construction
[2] https://docs.kernel.org/mm/damon/design.html#monitoring
[3] commit 5add26c0a186 ("mm/damon/core: remove damon_callback")
Thanks,
SJ
next prev parent reply other threads:[~2026-03-19 1:23 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-18 15:37 [Question] mm/damon: min_nr_regions constraint and inconsistent error handling Liew Rui Yan
2026-03-19 1:23 ` SeongJae Park [this message]
2026-03-19 1:46 ` SeongJae Park
2026-03-19 8:36 ` Liew Rui Yan
2026-03-19 15:15 ` 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=20260319012337.96726-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=aethernet65535@gmail.com \
--cc=damon@lists.linux.dev \
--cc=linux-mm@kvack.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