From: SeongJae Park <sj@kernel.org>
To: SeongJae Park <sj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
damon@lists.linux.dev, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Subject: Re: (sashiko review) [RFC PATCH v5 01/10] mm/damon/core: introduce damon_ctx->paused
Date: Mon, 23 Mar 2026 21:07:21 -0700 [thread overview]
Message-ID: <20260324040722.57944-1-sj@kernel.org> (raw)
In-Reply-To: <20260324012801.42930-1-sj@kernel.org>
On Mon, 23 Mar 2026 18:28:00 -0700 SeongJae Park <sj@kernel.org> wrote:
> Forwarding Sashiko review for doing discussions via mails.
>
> # review url: https://sashiko.dev/#/patchset/20260323231538.84452-2-sj@kernel.org
> # start of sashiko.dev inline review
> commit 011f7b451b6e4a657677ee09eada15a7980e8480
> Author: SeongJae Park <sj@kernel.org>
>
> mm/damon/core: introduce damon_ctx->paused
>
> This commit introduces a pause parameter to the DAMON context, allowing
> the kdamond main loop to be paused and resumed without losing its
> self-trained internal data.
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index db6c67e52d2b..0ab2cfa848e6 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -3003,6 +3004,14 @@ static int kdamond_fn(void *data)
> > kdamond_call(ctx, false);
> > if (ctx->maybe_corrupted)
> > break;
> > + while (ctx->pause) {
> > + damos_walk_cancel(ctx);
> > + kdamond_usleep(ctx->attrs.sample_interval);
> > + /* allow caller unset pause via damon_call() */
> > + kdamond_call(ctx, false);
> > + if (kdamond_need_stop(ctx) || ctx->maybe_corrupted)
> > + goto done;
> > + }
>
> Since this loop is located after ops.prepare_access_checks(), kdamond_usleep(),
> and ops.check_accesses(), does this unconditionally perform a full monitoring
> cycle before honoring the paused state?
>
> If a context is started with pause set to true, or if pause is enabled while
> waiting in kdamond_wait_activation(), it appears it will still execute one
> complete active sampling cycle. Could this cause unexpected disruption, such
> as clearing PTE accessed bits, when the context is explicitly configured to
> be paused?
Yes, DAMON would behave in the way. And having one more sampling work doesn't
cause a real issue.
>
> Additionally, when the context is paused, kdamond_fn remains in this sleep
> loop and skips calling damos_adjust_quota(), which means quota->charged_from
> is frozen.
>
> If the pause lasts for more than LONG_MAX jiffies (e.g., about 24.8 days on
> a 32-bit system with HZ=1000), will system jiffies wrap past the half-range
> limit of the unsigned long type relative to the frozen timestamp?
>
> If this wraparound occurs, time_after_eq(jiffies, quota->charged_from +
> interval) would evaluate to false when unpaused. Would this cause the quota
> window to not reset for another 24.8 days, accumulating charged_sz and
> blocking the scheme from being applied?
That's a wild corner case, but I agree it is better to avoid the problematic
case. I'm still thinking about the good way for that. Anyway, I will address
this in the next spin.
Thanks,
SJ
[...]
next prev parent reply other threads:[~2026-03-24 4:07 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-23 23:15 [RFC PATCH v5 00/10] mm/damon: let DAMON be paused and resumed SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 01/10] mm/damon/core: introduce damon_ctx->paused SeongJae Park
2026-03-24 1:28 ` (sashiko review) " SeongJae Park
2026-03-24 4:07 ` SeongJae Park [this message]
2026-04-02 1:10 ` SeongJae Park
2026-04-05 23:06 ` SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 02/10] mm/damon/sysfs: add pause file under context dir SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 03/10] Docs/mm/damon/design: update for context pause/resume feature SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 04/10] Docs/admin-guide/mm/damon/usage: update for pause file SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 05/10] Docs/ABI/damon: update for pause sysfs file SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 06/10] mm/damon/tests/core-kunit: test pause commitment SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 07/10] selftests/damon/_damon_sysfs: support pause file staging SeongJae Park
2026-03-24 1:28 ` (sashiko review) " SeongJae Park
2026-03-24 4:08 ` SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 08/10] selftests/damon/drgn_dump_damon_status: dump pause SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 09/10] selftests/damon/sysfs.py: check pause on assert_ctx_committed() SeongJae Park
2026-03-23 23:15 ` [RFC PATCH v5 10/10] selftests/damon/sysfs.py: pause DAMON before dumping status SeongJae Park
2026-03-24 1:27 ` (sashiko status) [RFC PATCH v5 00/10] mm/damon: let DAMON be paused and resumed 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=20260324040722.57944-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=damon@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--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 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.