All of lore.kernel.org
 help / color / mirror / Atom feed
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: [PATCH v2 0/2] mm/damon: reset thread status parameters upon kdamond termination
Date: Mon, 13 Apr 2026 17:22:59 -0700	[thread overview]
Message-ID: <20260414002300.83328-1-sj@kernel.org> (raw)
In-Reply-To: <20260413185249.5921-1-aethernet65535@gmail.com>

On Tue, 14 Apr 2026 02:52:47 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Problem
> ========

Let's align the underline with the subject.  Also, let's add one blank line
after the underline.

> When kdamond terminates unexpectedly, 'enabled' remains 'Y' and
> 'kdamond_pid' remains stale. This prevents user from restarting DAMON
> because both writing 'Y' and 'N' to 'enabled' will fail.
> 
> "Unexpected termination" here means the kdamond exits without any user
> request (e.g., not by writing 'N' to 'enabled').

Could you please further explain when such termination can happen?

> 
> User Impact
> ===========
> Once kdamond terminates this way, it cannot be restarted via sysfs
> because:
> 
> 1. DAMON_LRU_SORT/DAMON_RECLAIM is built into the kernel, so it cannot
>    be unloaded and reloaded at runtime.

I think this is quite obvious, so may better to be dropped.

> 2. Writing 'N' to 'enabled' fails because kdamond no longer exists;
>    Writing 'Y' does nothing, as 'enabled' is already Y.
> 
> As a result, the only way to restore DAMON functionality is a full
> system reboot.

Thank you for clarifying the user impact.  I think this deserves Cc-ing
stable@.

I think 'Problem' and 'User Impact' can be unified into one section.

> 
> Solution
> ========
> damon_commit_ctx() sets 'maybe_corrupted=true' at the beginning and only
> sets it to false upon successful completion. When 'maybe_corrupted'
> remains true, kdamond will terminate eventually.

This seems better to be explained earlier, on the problem section.

> 
> Therefore:
> 1. In damon_{lru_sort, reclaim}_turn(): Add fallback logic to reset
>    parameters when damon_stop() fails but kdamond is not running.
> 2. In damon_{lru_sort, reclaim}_apply_parameters(): Reset parameters
>    when damon_commit_ctx() fails, as kdamond will terminate due to
>    maybe_corrupted mechanism.

So the problem is that 'enable' parameter value is not trustworthy, and this
series is trying to make it trustworthy.  I think it is bit complicated,
especially for stable@ fix.  What about simply using more trustworthy
information, e.g.,

'''
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -390,7 +390,7 @@ MODULE_PARM_DESC(addr_unit,
 static int damon_reclaim_enabled_store(const char *val,
                const struct kernel_param *kp)
 {
-       bool is_enabled = enabled;
+       bool is_enabled = false;
        bool enable;
        int err;

@@ -398,6 +398,9 @@ static int damon_reclaim_enabled_store(const char *val,
        if (err)
                return err;

+       if (ctx)
+               is_enabled = damon_is_running(ctx);
+
        if (is_enabled == enable)
                return 0;

'''

> 
> Changes from RFC-v1
> (https://lore.kernel.org/20260330164347.12772-1-aethernet65535@gmail.com)
> - Remove RFC tag.

When dropping RFC tag, let's start from v1 again, from the next time.


Thanks,
SJ

[...]

  parent reply	other threads:[~2026-04-14  0:23 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-13 18:52 [PATCH v2 0/2] mm/damon: reset thread status parameters upon kdamond termination Liew Rui Yan
2026-04-13 18:52 ` [PATCH v2 1/2] mm/damon/lru_sort: " Liew Rui Yan
2026-04-13 19:54   ` (sashiko review) " Liew Rui Yan
2026-04-13 18:52 ` [PATCH v2 2/2] mm/damon/reclaim: " Liew Rui Yan
2026-04-13 19:57   ` (sashiko review) " Liew Rui Yan
2026-04-13 22:05 ` [PATCH v2 0/2] mm/damon: " Liew Rui Yan
2026-04-14  0:28   ` SeongJae Park
2026-04-14  0:22 ` SeongJae Park [this message]
2026-04-14  0:34   ` SeongJae Park
2026-04-15 18:45     ` Liew Rui Yan
2026-04-15 23:55       ` SeongJae Park
2026-04-16  6:28         ` SeongJae Park
2026-04-17  1:12           ` SeongJae Park
2026-04-17  4:27             ` Liew Rui Yan
2026-04-17 14:00               ` SeongJae Park
2026-04-16  0:17 ` 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=20260414002300.83328-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 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.