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

[SeongJae Park <sj@kernel.org>]

On Wed, 18 Mar 2026 18:23:36 -0700 SeongJae Park <sj@kernel.org> wrote:

> Hi Liew,
> 
> On Wed, 18 Mar 2026 23:37:31 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
[...]
> > 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...

I found it.  Commit 4c9ea539ad59 ("mm/damon/sysfs: validate user inputs from
damon_sysfs_commit_input()") discuss this.  To quote:

    Online DAMON parameters commit via DAMON sysfs interface can make kdamond
    stop.  This behavior was made because it can make the implementation
    simpler.  The implementation tries committing the parameter without
    validation.  If it finds something wrong in the middle of the parameters
    update, it returns error without reverting the partially committed
    parameters back.  It is safe though, since it immediately breaks kdamond
    main loop in the case of the error return.

    Users can make the wrong parameters by mistake, though.  Stopping kdamond
    in the case is not very useful behavior.  Also this makes it difficult to
    utilize damon_call() instead of damon_callback hook for online parameters
    update, since damon_call() cannot immediately break kdamond main loop in
    the middle.

    Validate the input parameters and return error when it fails before
    starting parameters updates.  In case of mistakenly wrong parameters,
    kdamond can continue running with the old and valid parameters.


Thanks,
SJ

[...]