Re: [Question] mm/damon: min_nr_regions constraint and inconsistent error handling

[SeongJae Park <sj@kernel.org>]

On Thu, 19 Mar 2026 16:36:39 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> On Thu, Mar 19, 2026 at 9:46 AM SeongJae Park <sj@kernel.org> wrote:
> 
> > Hi Liew,
> 
> Hi SeongJae,
> 
> > 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.
> 
> Thanks for your explanation! It makes perfect sense.
> 
> > 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.
> 
> Agreed. I will try to figure out which method is better,

Doing both may also make sense.

> and I might
> send out a patch in the near future.

Looking forward!

> 
> > 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 see. I understand your concern about log spam, I will focus on
> documentation.
> 
> > I believe the above command should returned an error.  If not, please let me
> > know.
> 
> I re-tested it on v7.0-rc4 and it did NOT return any error when writing
> 'Y' to 'commit_inputs' with an invalid 'min_nr_regions=1'.
> 
> This might be unintended. Here is the full log of my session:
> 
>     [root@virtme-ng x86_64]# cd /sys/module/damon_lru_sort/parameters/
>     [root@virtme-ng parameters]# cat enabled 
>     N
>     [root@virtme-ng parameters]# echo 65535 > max_nr_regions
>     [root@virtme-ng parameters]# echo 3 > min_nr_regions 
>     [root@virtme-ng parameters]# echo Y > enabled 
>     [root@virtme-ng parameters]# cat kdamond_pid 
>     89
>     [root@virtme-ng parameters]# echo 1 > min_nr_regions 
>     [root@virtme-ng parameters]# echo Y > commit_inputs 
>     [root@virtme-ng parameters]# cat kdamond_pid 
>     89
>     [root@virtme-ng parameters]# cat enabled 
>     Y
>     [root@virtme-ng parameters]# ps aux | rg "kdamond" | rg -v "rg"
>     root        89  0.0  0.0      0     0 ?        I    10:15   0:00 [kdamond.0]

Thank you for sharing this test results.  I also confirmed you are right.  My
above assumption (error on commit_inputs writing) is simply wrong.  In the case
of wrong argument writing, it just silently fails because the commit_inputs
handling is done in background.  So, not a bug.  But maybe something better to
be improved.  I think it is better to return an explicit error and/or document
the behavior in near future.

> 
> > Also, if you know users who depend on the old behavior and therefore hoping the
> > old behavior back, please let me know.
> 
> I don't know of anyone who depends on the old behavior.
> I am a student exploring the kernel (DAMON) as a hobby, so my
> perspective is limited to individual research rather than large-scale
> production environments.

Thank you for sharing the context.  People from different background gives us
very different and important perspectives.  I appreciate all such voices.  And
knowing the context of from where it was made is very helpful at discussing
suggested changes.  Thank you again.

> However, the current behavior (staying alive
> with valid params) feels much more sensible to me. But it should return
> an error when 'commit_inputs=Y'.

I agree.  Maybe something that we can improve.

> 
> ---
> 
> Plan
> ====
> Based on our discussion, my current plan is to:
> 1. Document the 'min_nr_regions' constraint in admin-guide.
> 2. Add the design rationale for the "at least 3 regions" rule in
>    design.rst.
> 3. Remove the outdated sentences claiming DAMON will disable itself on
>    invalid parameters input.

I found an issue in the current behavior, and just posted a fix.  Assuming the
fix is merged, the behavior will again be changed.  So, please do this only
after the fix is merged or completely abandoned.

If the fix is merged, the changed behavior will be something like,

"It will continue running with old valid parameters if the parameters are
wrong.  It will stop running if the parameters update was unable to be
completed due to internal error."

> 4. Investigate and try to fix the missing error return for
>    'commit_inputs'.
> 
> Does this plan cover everything you'd expect?

Yes, this sounds like a very nice plan.  I'm looking forward to your next mail!
:)


Thanks,
SJ

[...]