* [PATCH 6.1.y 1/2] mm/damon/core: implement damon_kdamond_pid()
[not found] <2026051243-crevice-spool-75d8@gregkh>
@ 2026-05-13 4:30 ` SeongJae Park
2026-05-14 1:39 ` sashiko-bot
2026-05-13 4:30 ` [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values SeongJae Park
1 sibling, 1 reply; 7+ messages in thread
From: SeongJae Park @ 2026-05-13 4:30 UTC (permalink / raw)
To: stable; +Cc: SeongJae Park, damon, Andrew Morton
Patch series "mm/damon: hide kdamond and kdamond_lock from API callers".
'kdamond' and 'kdamond_lock' fields initially exposed to DAMON API callers
for flexible synchronization and use cases. As DAMON API became somewhat
complicated compared to the early days, Keeping those exposed could only
encourage the API callers to invent more creative but complicated and
difficult-to-debug use cases.
Fortunately DAMON API callers didn't invent that many creative use cases.
There exist only two use cases of 'kdamond' and 'kdamond_lock'. Finding
whether the kdamond is actively running, and getting the pid of the
kdamond. For the first use case, a dedicated API function, namely
'damon_is_running()' is provided, and all DAMON API callers are using the
function for the use case. Hence only the second use case is where the
fields are directly being used by DAMON API callers.
To prevent future invention of complicated and erroneous use cases of the
fields, hide the fields from the API callers. For that, provide new
dedicated DAMON API functions for the remaining use case, namely
damon_kdamond_pid(), migrate DAMON API callers to use the new function,
and mark the fields as private fields.
This patch (of 5):
'kdamond' and 'kdamond_lock' are directly being used by DAMON API callers
for getting the pid of the corresponding kdamond. To discourage invention
of creative but complicated and erroneous new usages of the fields that
require careful synchronization, implement a new API function that can
simply be used without the manual synchronizations.
Link: https://lkml.kernel.org/r/20260115152047.68415-1-sj@kernel.org
Link: https://lkml.kernel.org/r/20260115152047.68415-2-sj@kernel.org
Signed-off-by: SeongJae Park <sj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 4262c53236977de3ceaa3bf2aefdf772c9b874dd)
Signed-off-by: SeongJae Park <sj@kernel.org>
---
The second patch depends on this commit, so porting this together.
include/linux/damon.h | 1 +
mm/damon/core.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index e6941b239f449..7eeec0eaaf1f5 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -572,6 +572,7 @@ static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs
int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive);
int damon_stop(struct damon_ctx **ctxs, int nr_ctxs);
+int damon_kdamond_pid(struct damon_ctx *ctx);
int damon_set_region_biggest_system_ram_default(struct damon_target *t,
unsigned long *start, unsigned long *end);
diff --git a/mm/damon/core.c b/mm/damon/core.c
index ab5c351b276ce..fc68364c9ad2e 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -643,6 +643,23 @@ static bool damon_check_reset_time_interval(struct timespec64 *baseline,
return true;
}
+/**
+ * damon_kdamond_pid() - Return pid of a given DAMON context's worker thread.
+ * @ctx: The DAMON context of the question.
+ *
+ * Return: pid if @ctx is running, negative error code otherwise.
+ */
+int damon_kdamond_pid(struct damon_ctx *ctx)
+{
+ int pid = -EINVAL;
+
+ mutex_lock(&ctx->kdamond_lock);
+ if (ctx->kdamond)
+ pid = ctx->kdamond->pid;
+ mutex_unlock(&ctx->kdamond_lock);
+ return pid;
+}
+
/*
* Check whether it is time to flush the aggregated information
*/
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
[not found] <2026051243-crevice-spool-75d8@gregkh>
2026-05-13 4:30 ` [PATCH 6.1.y 1/2] mm/damon/core: implement damon_kdamond_pid() SeongJae Park
@ 2026-05-13 4:30 ` SeongJae Park
2026-05-14 2:01 ` sashiko-bot
2026-05-14 5:29 ` SeongJae Park
1 sibling, 2 replies; 7+ messages in thread
From: SeongJae Park @ 2026-05-13 4:30 UTC (permalink / raw)
To: stable; +Cc: SeongJae Park, damon, Liew Rui Yan, Andrew Morton
DAMON_LRU_SORT updates 'enabled' and 'kdamond_pid' parameter values, which
represents the running status of its kdamond, when the user explicitly
requests start/stop of the kdamond. The kdamond can, however, be stopped
in events other than the explicit user request in the following three
events.
1. ctx->regions_score_histogram allocation failure at beginning of the
execution,
2. damon_commit_ctx() failure due to invalid user input, and
3. damon_commit_ctx() failure due to its internal allocation failures.
Hence, if the kdamond is stopped by the above three events, the values of
the status parameters can be stale. Users could show the stale values and
be confused. This is already bad, but the real consequence is worse.
DAMON_LRU_SORT avoids unnecessary damon_start() and damon_stop() calls
based on the 'enabled' parameter value. And the update of 'enabled'
parameter value depends on the damon_start() and damon_stop() call
results. Hence, once the kdamond has stopped by the unintentional events,
the user cannot restart the kdamond before the system reboot. For
example, the issue can be reproduced via below steps.
# cd /sys/module/damon_lru_sort/parameters
#
# # start DAMON_LRU_SORT
# echo Y > enabled
# ps -ef | grep kdamond
root 806 2 0 17:53 ? 00:00:00 [kdamond.0]
root 808 803 0 17:53 pts/4 00:00:00 grep kdamond
#
# # commit wrong input to stop kdamond withou explicit stop request
# echo 3 > addr_unit
# echo Y > commit_inputs
bash: echo: write error: Invalid argument
#
# # confirm kdamond is stopped
# ps -ef | grep kdamond
root 811 803 0 17:53 pts/4 00:00:00 grep kdamond
#
# # users casn now show stable status
# cat enabled
Y
# cat kdamond_pid
806
#
# # even after fixing the wrong parameter,
# # kdamond cannot be restarted.
# echo 1 > addr_unit
# echo Y > enabled
# ps -ef | grep kdamond
root 815 803 0 17:54 pts/4 00:00:00 grep kdamond
The problem will only rarely happen in real and common setups for the
following reasons. The allocation failures are unlikely in such setups
since those allocations are arguably too small to fail. Also sane users
on real production environments may not commit wrong input parameters.
But once it happens, the consequence is quite bad. And the bug is a bug.
The issue stems from the fact that there are multiple events that can
change the status, and following all the events is challenging.
Dynamically detect and use the fresh status for the parameters when those
are requested.
Link: https://lore.kernel.org/20260419161003.79176-3-sj@kernel.org
Fixes: 40e983cca927 ("mm/damon: introduce DAMON-based LRU-lists Sorting")
Co-developed-by: Liew Rui Yan <aethernet65535@gmail.com>
Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
Signed-off-by: SeongJae Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 6.0.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit b98b7ff6025ae82570d4915e083f0cbd8d48b3cf)
Signed-off-by: SeongJae Park <sj@kernel.org>
(port parts of 42b7491af14c ("mm/damon/core: introduce damon_call()")
and d2b5be741a50 ("mm/damon/sysfs: use DAMON core API
damon_is_running()") for damon_is_running() dependency)
---
include/linux/damon.h | 1 +
mm/damon/core.c | 16 ++++++++++
mm/damon/lru_sort.c | 74 +++++++++++++++++++++++++++++++------------
3 files changed, 70 insertions(+), 21 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 7eeec0eaaf1f5..7133131433547 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -572,6 +572,7 @@ static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs
int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive);
int damon_stop(struct damon_ctx **ctxs, int nr_ctxs);
+bool damon_is_running(struct damon_ctx *ctx);
int damon_kdamond_pid(struct damon_ctx *ctx);
int damon_set_region_biggest_system_ram_default(struct damon_target *t,
diff --git a/mm/damon/core.c b/mm/damon/core.c
index fc68364c9ad2e..1c041d011b8ef 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -643,6 +643,22 @@ static bool damon_check_reset_time_interval(struct timespec64 *baseline,
return true;
}
+/**
+ * damon_is_running() - Returns if a given DAMON context is running.
+ * @ctx: The DAMON context to see if running.
+ *
+ * Return: true if @ctx is running, false otherwise.
+ */
+bool damon_is_running(struct damon_ctx *ctx)
+{
+ bool running;
+
+ mutex_lock(&ctx->kdamond_lock);
+ running = ctx->kdamond != NULL;
+ mutex_unlock(&ctx->kdamond_lock);
+ return running;
+}
+
/**
* damon_kdamond_pid() - Return pid of a given DAMON context's worker thread.
* @ctx: The DAMON context of the question.
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 61311800abc98..70cdc7425cdd3 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -113,15 +113,6 @@ module_param(monitor_region_start, ulong, 0600);
static unsigned long monitor_region_end __read_mostly;
module_param(monitor_region_end, ulong, 0600);
-/*
- * PID of the DAMON thread
- *
- * If DAMON_LRU_SORT is enabled, this becomes the PID of the worker thread.
- * Else, -1.
- */
-static int kdamond_pid __read_mostly = -1;
-module_param(kdamond_pid, int, 0400);
-
static struct damos_stat damon_lru_sort_hot_stat;
DEFINE_DAMON_MODULES_DAMOS_STATS_PARAMS(damon_lru_sort_hot_stat,
lru_sort_tried_hot_regions, lru_sorted_hot_regions,
@@ -249,22 +240,14 @@ static int damon_lru_sort_turn(bool on)
{
int err;
- if (!on) {
- err = damon_stop(&ctx, 1);
- if (!err)
- kdamond_pid = -1;
- return err;
- }
+ if (!on)
+ return damon_stop(&ctx, 1);
err = damon_lru_sort_apply_parameters();
if (err)
return err;
- err = damon_start(&ctx, 1, true);
- if (err)
- return err;
- kdamond_pid = ctx->kdamond->pid;
- return 0;
+ return damon_start(&ctx, 1, true);
}
static struct delayed_work damon_lru_sort_timer;
@@ -285,6 +268,13 @@ static DECLARE_DELAYED_WORK(damon_lru_sort_timer, damon_lru_sort_timer_fn);
static bool damon_lru_sort_initialized;
+static bool damon_lru_sort_enabled(void)
+{
+ if (!ctx)
+ return false;
+ return damon_is_running(ctx);
+}
+
static int damon_lru_sort_enabled_store(const char *val,
const struct kernel_param *kp)
{
@@ -301,15 +291,57 @@ static int damon_lru_sort_enabled_store(const char *val,
return 0;
}
+static int damon_lru_sort_enabled_load(char *buffer,
+ const struct kernel_param *kp)
+{
+ return sprintf(buffer, "%c\n", damon_lru_sort_enabled() ? 'Y' : 'N');
+}
+
static const struct kernel_param_ops enabled_param_ops = {
.set = damon_lru_sort_enabled_store,
- .get = param_get_bool,
+ .get = damon_lru_sort_enabled_load,
};
module_param_cb(enabled, &enabled_param_ops, &enabled, 0600);
MODULE_PARM_DESC(enabled,
"Enable or disable DAMON_LRU_SORT (default: disabled)");
+static int damon_lru_sort_kdamond_pid_store(const char *val,
+ const struct kernel_param *kp)
+{
+ /*
+ * kdamond_pid is read-only, but kernel command line could write it.
+ * Do nothing here.
+ */
+ return 0;
+}
+
+static int damon_lru_sort_kdamond_pid_load(char *buffer,
+ const struct kernel_param *kp)
+{
+ int kdamond_pid = -1;
+
+ if (ctx) {
+ kdamond_pid = damon_kdamond_pid(ctx);
+ if (kdamond_pid < 0)
+ kdamond_pid = -1;
+ }
+ return sprintf(buffer, "%d\n", kdamond_pid);
+}
+
+static const struct kernel_param_ops kdamond_pid_param_ops = {
+ .set = damon_lru_sort_kdamond_pid_store,
+ .get = damon_lru_sort_kdamond_pid_load,
+};
+
+/*
+ * PID of the DAMON thread
+ *
+ * If DAMON_LRU_SORT is enabled, this becomes the PID of the worker thread.
+ * Else, -1.
+ */
+module_param_cb(kdamond_pid, &kdamond_pid_param_ops, NULL, 0400);
+
static int damon_lru_sort_handle_commit_inputs(void)
{
int err;
--
2.47.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 6.1.y 1/2] mm/damon/core: implement damon_kdamond_pid()
2026-05-13 4:30 ` [PATCH 6.1.y 1/2] mm/damon/core: implement damon_kdamond_pid() SeongJae Park
@ 2026-05-14 1:39 ` sashiko-bot
2026-05-14 4:43 ` SeongJae Park
0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-05-14 1:39 UTC (permalink / raw)
To: SeongJae Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message references a non-existent function `damon_is_running()` and makes inaccurate claims about the current state of API callers.
--
commit 79a54a37fc83e2191edada566d0eb1ab79303fd8
Author: SeongJae Park <sj@kernel.org>
mm/damon/core: implement damon_kdamond_pid()
This commit hides the kdamond and kdamond_lock fields from DAMON API callers
by implementing a new damon_kdamond_pid() function to retrieve the worker
thread's PID.
Link: https://lkml.kernel.org/r/20260115152047.68415-1-sj@kernel.org
Link: https://lkml.kernel.org/r/20260115152047.68415-2-sj@kernel.org
> For the first use case, a dedicated API function, namely
> 'damon_is_running()' is provided, and all DAMON API callers are using the
> function for the use case.
Is damon_is_running() actually present in the codebase at this point?
The commit message states that damon_is_running() is provided and all API
callers are using it, but it appears this function does not exist in the
codebase at the time of this commit. It seems to only be introduced in the
subsequent patch "mm/damon/lru_sort: detect and use fresh enabled and
kdamond_pid values" as a backported dependency.
Furthermore, API callers like mm/damon/sysfs.c (via
damon_sysfs_ctx_running()) still directly check ctx->kdamond != NULL under
kdamond_lock.
Could this description be inaccurate, perhaps due to patches being applied
out of order during a backport?
--
Sashiko AI review · https://sashiko.dev/#/patchset/2026051243-crevice-spool-75d8@gregkh?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
2026-05-13 4:30 ` [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values SeongJae Park
@ 2026-05-14 2:01 ` sashiko-bot
2026-05-14 5:26 ` SeongJae Park
2026-05-14 5:29 ` SeongJae Park
1 sibling, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-05-14 2:01 UTC (permalink / raw)
To: SeongJae Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch completely fails to fix the restart issue, leaving the state machine deadlocked.
- [High] Use-After-Free when reading sysfs parameters if module initialization fails.
--
commit 975a161fef47e6bccc8ce6810404c2d8be4ff338
Author: SeongJae Park <sj@kernel.org>
Subject: mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
This commit updates the DAMON_LRU_SORT module to dynamically detect and report
the fresh status of enabled and kdamond_pid parameters. It attempts to address
an issue where unintentional kdamond stoppages leave the system in a stale
state, preventing users from restarting the daemon. By directly calling
damon_is_running() and damon_kdamond_pid(), the system aims to provide
accurate runtime information to sysfs readers.
Link: https://lore.kernel.org/20260419161003.79176-3-sj@kernel.org
> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 61311800abc98..70cdc7425cdd3 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
[ ... ]
> @@ -285,6 +268,13 @@ static DECLARE_DELAYED_WORK(damon_lru_sort_timer, damon_lru_sort_timer_fn);
>
> static bool damon_lru_sort_initialized;
>
> +static bool damon_lru_sort_enabled(void)
> +{
> + if (!ctx)
> + return false;
> + return damon_is_running(ctx);
> +}
Is it possible for this to cause a use-after-free if module initialization
fails?
If damon_lru_sort_init() encounters an error (for example, damon_select_ops()
fails), it calls damon_destroy_ctx(ctx) to free the context. However, the
global static ctx pointer is not set to NULL.
Since DAMON_LRU_SORT is a built-in module, its module parameters are exposed
in sysfs early during boot. If a user reads the sysfs parameters after an init
failure, the dangling ctx pointer would pass the check here, leading to
damon_is_running() dereferencing it:
mm/damon/core.c:damon_is_running()
mutex_lock(&ctx->kdamond_lock);
[ ... ]
> @@ -301,15 +291,57 @@ static int damon_lru_sort_enabled_store(const char *val,
> return 0;
> }
>
> +static int damon_lru_sort_enabled_load(char *buffer,
> + const struct kernel_param *kp)
> +{
> + return sprintf(buffer, "%c\n", damon_lru_sort_enabled() ? 'Y' : 'N');
> +}
> +
> static const struct kernel_param_ops enabled_param_ops = {
> .set = damon_lru_sort_enabled_store,
> - .get = param_get_bool,
> + .get = damon_lru_sort_enabled_load,
> };
Does this change actually fix the restart issue, or does it leave the state
machine deadlocked?
If kdamond stops unintentionally, the internal enabled and last_enabled
variables remain true. When the user tries to restart it by writing 'Y' to the
sysfs enabled parameter, the underlying boolean value remains true. The worker
skips the restart because last_enabled == now_enabled.
If the user tries to reset the state by writing 'N',
damon_lru_sort_turn(false) calls damon_stop() which returns an error because
the daemon is already stopped. Because of this error, the worker executes
enabled = last_enabled, reverting enabled back to true:
mm/damon/lru_sort.c:damon_lru_sort_timer_fn()
if (!damon_lru_sort_turn(now_enabled))
last_enabled = now_enabled;
else
enabled = last_enabled;
It seems the subsystem still cannot be restarted without a reboot.
--
Sashiko AI review · https://sashiko.dev/#/patchset/2026051243-crevice-spool-75d8@gregkh?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 6.1.y 1/2] mm/damon/core: implement damon_kdamond_pid()
2026-05-14 1:39 ` sashiko-bot
@ 2026-05-14 4:43 ` SeongJae Park
0 siblings, 0 replies; 7+ messages in thread
From: SeongJae Park @ 2026-05-14 4:43 UTC (permalink / raw)
To: sashiko-bot; +Cc: SeongJae Park, damon
On Thu, 14 May 2026 01:39:23 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The commit message references a non-existent function `damon_is_running()` and makes inaccurate claims about the current state of API callers.
> --
>
> commit 79a54a37fc83e2191edada566d0eb1ab79303fd8
> Author: SeongJae Park <sj@kernel.org>
>
> mm/damon/core: implement damon_kdamond_pid()
>
> This commit hides the kdamond and kdamond_lock fields from DAMON API callers
> by implementing a new damon_kdamond_pid() function to retrieve the worker
> thread's PID.
>
> Link: https://lkml.kernel.org/r/20260115152047.68415-1-sj@kernel.org
> Link: https://lkml.kernel.org/r/20260115152047.68415-2-sj@kernel.org
>
> > For the first use case, a dedicated API function, namely
> > 'damon_is_running()' is provided, and all DAMON API callers are using the
> > function for the use case.
>
> Is damon_is_running() actually present in the codebase at this point?
>
> The commit message states that damon_is_running() is provided and all API
> callers are using it, but it appears this function does not exist in the
> codebase at the time of this commit. It seems to only be introduced in the
> subsequent patch "mm/damon/lru_sort: detect and use fresh enabled and
> kdamond_pid values" as a backported dependency.
>
> Furthermore, API callers like mm/damon/sysfs.c (via
> damon_sysfs_ctx_running()) still directly check ctx->kdamond != NULL under
> kdamond_lock.
>
> Could this description be inaccurate, perhaps due to patches being applied
> out of order during a backport?
Sashiko is correct, this description is inaccurate. But that's intentional for
stable kernel backports.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/2026051243-crevice-spool-75d8@gregkh?part=1
Thanks,
SJ
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
2026-05-14 2:01 ` sashiko-bot
@ 2026-05-14 5:26 ` SeongJae Park
0 siblings, 0 replies; 7+ messages in thread
From: SeongJae Park @ 2026-05-14 5:26 UTC (permalink / raw)
To: sashiko-bot; +Cc: SeongJae Park, damon
On Thu, 14 May 2026 02:01:45 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The patch completely fails to fix the restart issue, leaving the state machine deadlocked.
> - [High] Use-After-Free when reading sysfs parameters if module initialization fails.
> --
>
> commit 975a161fef47e6bccc8ce6810404c2d8be4ff338
> Author: SeongJae Park <sj@kernel.org>
> Subject: mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
>
> This commit updates the DAMON_LRU_SORT module to dynamically detect and report
> the fresh status of enabled and kdamond_pid parameters. It attempts to address
> an issue where unintentional kdamond stoppages leave the system in a stale
> state, preventing users from restarting the daemon. By directly calling
> damon_is_running() and damon_kdamond_pid(), the system aims to provide
> accurate runtime information to sysfs readers.
>
> Link: https://lore.kernel.org/20260419161003.79176-3-sj@kernel.org
>
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index 61311800abc98..70cdc7425cdd3 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
>
> [ ... ]
>
> > @@ -285,6 +268,13 @@ static DECLARE_DELAYED_WORK(damon_lru_sort_timer, damon_lru_sort_timer_fn);
> >
> > static bool damon_lru_sort_initialized;
> >
> > +static bool damon_lru_sort_enabled(void)
> > +{
> > + if (!ctx)
> > + return false;
> > + return damon_is_running(ctx);
> > +}
>
> Is it possible for this to cause a use-after-free if module initialization
> fails?
>
> If damon_lru_sort_init() encounters an error (for example, damon_select_ops()
> fails),
So, damon_lru_sort_init() callse damon_modules_new_paddr_ctx_target(), which
calls damon_select_ops(). The damon_select_ops() cannot fail because
DAMON_LRU_SORT depends on DAMON_PADDR, and the damon_select_ops() is for paddr
ops setup.
But, damon_modules_new_paddr_ctx_target() calls damon_new_target() after that.
It could theoretically fail due to an allocation failure.
> it calls damon_destroy_ctx(ctx) to free the context. However, the
> global static ctx pointer is not set to NULL.
>
> Since DAMON_LRU_SORT is a built-in module, its module parameters are exposed
> in sysfs early during boot. If a user reads the sysfs parameters after an init
> failure, the dangling ctx pointer would pass the check here, leading to
> damon_is_running() dereferencing it:
>
> mm/damon/core.c:damon_is_running()
> mutex_lock(&ctx->kdamond_lock);
Makes sense. I will work on this. Not a blocker of this patch though.
>
> [ ... ]
>
> > @@ -301,15 +291,57 @@ static int damon_lru_sort_enabled_store(const char *val,
> > return 0;
> > }
> >
> > +static int damon_lru_sort_enabled_load(char *buffer,
> > + const struct kernel_param *kp)
> > +{
> > + return sprintf(buffer, "%c\n", damon_lru_sort_enabled() ? 'Y' : 'N');
> > +}
> > +
> > static const struct kernel_param_ops enabled_param_ops = {
> > .set = damon_lru_sort_enabled_store,
> > - .get = param_get_bool,
> > + .get = damon_lru_sort_enabled_load,
> > };
>
> Does this change actually fix the restart issue, or does it leave the state
> machine deadlocked?
>
> If kdamond stops unintentionally, the internal enabled and last_enabled
> variables remain true. When the user tries to restart it by writing 'Y' to the
> sysfs enabled parameter, the underlying boolean value remains true. The worker
> skips the restart because last_enabled == now_enabled.
>
> If the user tries to reset the state by writing 'N',
> damon_lru_sort_turn(false) calls damon_stop() which returns an error because
> the daemon is already stopped. Because of this error, the worker executes
> enabled = last_enabled, reverting enabled back to true:
>
> mm/damon/lru_sort.c:damon_lru_sort_timer_fn()
> if (!damon_lru_sort_turn(now_enabled))
> last_enabled = now_enabled;
> else
> enabled = last_enabled;
>
> It seems the subsystem still cannot be restarted without a reboot.
Correct, damon_lru_sort_enabled_store() part upstream change is not properly
ported. I will post a new version.
Thanks,
SJ
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/2026051243-crevice-spool-75d8@gregkh?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
2026-05-13 4:30 ` [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values SeongJae Park
2026-05-14 2:01 ` sashiko-bot
@ 2026-05-14 5:29 ` SeongJae Park
1 sibling, 0 replies; 7+ messages in thread
From: SeongJae Park @ 2026-05-14 5:29 UTC (permalink / raw)
To: SeongJae Park; +Cc: stable, damon, Liew Rui Yan, Andrew Morton
On Tue, 12 May 2026 21:30:36 -0700 SeongJae Park <sj@kernel.org> wrote:
[...]
> Link: https://lore.kernel.org/20260419161003.79176-3-sj@kernel.org
> Fixes: 40e983cca927 ("mm/damon: introduce DAMON-based LRU-lists Sorting")
> Co-developed-by: Liew Rui Yan <aethernet65535@gmail.com>
> Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
> Signed-off-by: SeongJae Park <sj@kernel.org>
> Cc: <stable@vger.kernel.org> # 6.0.x
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> (cherry picked from commit b98b7ff6025ae82570d4915e083f0cbd8d48b3cf)
> Signed-off-by: SeongJae Park <sj@kernel.org>
> (port parts of 42b7491af14c ("mm/damon/core: introduce damon_call()")
> and d2b5be741a50 ("mm/damon/sysfs: use DAMON core API
> damon_is_running()") for damon_is_running() dependency)
Sashiko found this backport is incomplete. Please read my reply [1] for
details. I will send a complete version as a reply to this mail.
[1] https://lore.kernel.org/20260514052626.111769-1-sj@kernel.org
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-14 5:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <2026051243-crevice-spool-75d8@gregkh>
2026-05-13 4:30 ` [PATCH 6.1.y 1/2] mm/damon/core: implement damon_kdamond_pid() SeongJae Park
2026-05-14 1:39 ` sashiko-bot
2026-05-14 4:43 ` SeongJae Park
2026-05-13 4:30 ` [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values SeongJae Park
2026-05-14 2:01 ` sashiko-bot
2026-05-14 5:26 ` SeongJae Park
2026-05-14 5:29 ` SeongJae Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox