mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: Aveline Noir <jm5905938@gmail.com>
Cc: SJ Park <sj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	damon@lists.linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] mm/damon/core: reset watermark wait state on each iteration
Date: Fri, 23 Oct 2026 18:22:27 -0700	[thread overview]
Message-ID: <20261024012228.88920-1-sj@kernel.org> (raw)
In-Reply-To: <179277129124.2.16132047926074873101.damon-empty-schemes@gmail.com>

On Sat, 24 Oct 2026 00:01:31 +0800 Aveline Noir <jm5905938@gmail.com> wrote:

> Found while reviewing the DAMON source code that if an online commit 
> removes all schemes while kdamond is waiting for watermark activation,
> the scheme loop becomes empty but min_wait_time keeps the previous 
> positive value.  kdamond then keeps sleeping instead of resuming 
> monitoring without schemes.
> 
> Initialize min_wait_time and init_wait_time on each loop iteration so
> the wait is calculated from the current schemes and an empty scheme
> list resumes monitoring.

Nice catch and fix, thank you Aveline!

> 
> Fixes: 6e74d2bf5a26 ("mm/damon/core: add a new callback for watermarks checks")

But, this is a preparational patch for online commit support, right?  Shouldn't
the Fixes: be the commit that enabled online commit?

Also, this bug seems nasty to me.  Users could work around the issue without
restarting DAMON only by installing a scheme that has watermarks that activates
DAMON again, and then removing the scheme again.  I think it deserves to add
this to commit message and Cc stable@.

Fixes: da87878010e5 ("mm/damon/sysfs: support online inputs update")
Cc: <stable@vger.kernel.org> # 5.19.x

> Assisted-by: LLM
> Signed-off-by: Aveline Noir <jm5905938@gmail.com>

Reviewed-by: SJ Park <sj@kernel.org>

> ---
> An x86-64 KVM test using free-memory watermarks reproduced the stall
> with nr_schemes=0 followed by commit.  The same test resumed monitoring
> with this fix.  DAMON KUnit and selftests all passed.

Thank you for keep adding this useful test results.

Assuming you agree to above suggestions, I applied  this patch to
damon/next [1] tree after applying my suggestions like below.  We are in the
middle of the merge window.
It may better to let mm.git maintainer (Andrew Morton) take time for pull
request works, rather than adding more changes that are not really urgent.  I
understand this series is not really urgent.  Hence, Andrew might not add this
patch until next -rc1 release.  In the case, I will request adding this to
mm.git after the next -rc1 release.  So, no action from your side is needed for
now.  Let me know if you find anything I missed, or disagree to the suggestions
or the urgency judgement.

Changes to the patch file that I made:

'''
@@ -8,6 +8,10 @@ the scheme loop becomes empty but min_wait_time keeps the previous
 positive value.  kdamond then keeps sleeping instead of resuming
 monitoring without schemes.

+The way to wake up the kdamond again without DAMON is bit complicated.
+Install a scheme having watermarks that can activate the kdamond again,
+and remove the scheme again.  Doable but cumbersome and complicated.
+
 Initialize min_wait_time and init_wait_time on each loop iteration so
 the wait is calculated from the current schemes and an empty scheme
 list resumes monitoring.
@@ -15,11 +19,15 @@ list resumes monitoring.
 /* TODO: move below to commentary */
 Changes from v1
 - v1: https://lore.kernel.org/179277129124.2.16132047926074873101.damon-empty-schemes@gmail.com
-- None.
+- Collect R-b: from SJ.
+- Add how to work around the problem.
+- Update Fixes: and add Cc: stable@.

-Fixes: 6e74d2bf5a26 ("mm/damon/core: add a new callback for watermarks checks")
+Fixes: da87878010e5 ("mm/damon/sysfs: support online inputs update")
+Cc: <stable@vger.kernel.org> # 5.19.x
 Assisted-by: LLM
 Signed-off-by: Aveline Noir <jm5905938@gmail.com>
+Reviewed-by: SJ Park <sj@kernel.org>
 Signed-off-by: SJ Park <sj@kernel.org>
'''

[1] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees


Thanks,
SJ

[...]

      reply	other threads:[~2026-10-24  1:22 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23 16:01 Aveline Noir
2026-10-24  1:22 ` SJ Park [this message]

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=20261024012228.88920-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=damon@lists.linux.dev \
    --cc=jm5905938@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stable@vger.kernel.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

all inboxes | Powered by JetHome®