FAILED: patch "[PATCH] mm/damon/lru_sort: detect and use fresh enabled and" failed to apply to 6.1-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x b98b7ff6025ae82570d4915e083f0cbd8d48b3cf
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026051243-crevice-spool-75d8@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From b98b7ff6025ae82570d4915e083f0cbd8d48b3cf Mon Sep 17 00:00:00 2001
From: SeongJae Park <[email protected]>
Date: Sun, 19 Apr 2026 09:10:01 -0700
Subject: [PATCH] mm/damon/lru_sort: detect and use fresh enabled and
kdamond_pid values
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 kdamondThe 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.
mm/damon/lru_sort.c +55 -30
@@ -161,15 +161,6 @@ module_param(monitor_region_end, ulong, 0600);*/ static unsigned long addr_unit__read_mostly =1 ; -/* -* 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,
@@ -386,12 +377,8 @@ 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)
@@ -400,9 +387,6 @@ static int damon_lru_sort_turn(bool on) err =damon_start (&ctx,1 , true);if (err)return err; - kdamond_pid =damon_kdamond_pid (ctx); -if (kdamond_pid <0 ) -return kdamond_pid;return damon_call (ctx, &call_control); }
@@ -430,42 +414,83 @@ module_param_cb(addr_unit, &addr_unit_param_ops, &addr_unit, 0600);MODULE_PARM_DESC (addr_unit,"Scale factor for DAMON_LRU_SORT to ops address conversion (default: 1)" ); +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) { -bool is_enabled = enabled; -bool enable;int err; - err =kstrtobool (val, &enable); + err =kstrtobool (val, &enabled);if (err)return err; -if (is_enabled == enable) +if (damon_lru_sort_enabled () == enabled)return 0 ;/* Called before init function. The function will handle this. */ if (!damon_initialized ()) - goto set_param_out; +return 0 ; - err =damon_lru_sort_turn (enable); -if (err) -return err; +return damon_lru_sort_turn (enabled); +} -set_param_out: - enabled = enable; -return err; +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 __init damon_lru_sort_init (void ) {int err;
SeongJae Park
maintainerPATCH 1/2 mm/damon/core: implement damon_kdamond_pid()
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.
---
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(+)
include/linux/damon.h +1 -0
@@ -572,6 +572,7 @@ static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrsint 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);
mm/damon/core.c +17 -0
@@ -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 */
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 6.1.y 1/2] mm/damon/core: implement damon_kdamond_pid()
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…
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 <[email protected]>
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.
Quoted 3 lines
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?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/2026051243-crevice-spool-75d8@gregkh?part=1
SeongJae Park
maintainerRe: [PATCH 6.1.y 1/2] mm/damon/core: implement damon_kdamond_pid()
Quoted 28 lines · On Thu, 14 May 2026 01:39:23 +0000 [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message references a non-existent functiondamon_is_running()and makes inaccurate claims about the current state of API callers. --commit
79a54a37fc83e2191edada566d0eb1ab79303fd8
Author: SeongJae Park <[email protected]>
mm/damon/core: implementdamon_kdamond_pid()This commit hides the kdamond and
kdamond_lockfields from DAMON API callers by implementing a newdamon_kdamond_pid()function to retrieve the worker thread's PID.Quoted 3 lines
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 andkdamond_pidvalues" as a backported dependency.
Quoted 6 lines
Furthermore, API callers like
mm/damon/sysfs.c(viadamon_sysfs_ctx_running()) still directly checkctx->kdamond!= NULL underkdamond_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.
Quoted 3 lines
Signature · 2 lines
-- Sashiko AI review · https://sashiko.dev/#/patchset/2026051243-crevice-spool-75d8@gregkh?part=1
Thanks,
SJ
SeongJae Park
maintainerPATCH 2/2 mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
In reply to [email protected]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 kdamondThe 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.
(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(-)
include/linux/damon.h +1 -0
@@ -572,6 +572,7 @@ static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrsint 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,
mm/damon/core.c +16 -0
@@ -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.
mm/damon/lru_sort.c +53 -21
111 lines not on this page · raw message
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
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.…
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 <[email protected]>
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.
Quoted 4 lines
mm/damon/lru_sort.c +0
-0
[ ... ]
mm/damon/lru_sort.c static DECLARE_DELAYED_WORK(damon_lru_sort_timer, damon_lru_sort_timer_fn); · line 276
Quoted 10 lines
mm/damon/lru_sort.c +6
-0@@ -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);
[ ... ]
Quoted 5 lines
mm/damon/lru_sort.c +1
-0@@ -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,
mm/damon/lru_sort.c static int damon_lru_sort_enabled_store(const char *val, · line 303
Quoted 10 lines
mm/damon/lru_sort.c +6
-1+ 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.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/2026051243-crevice-spool-75d8@gregkh?part=2
SeongJae Park
maintainerRe: [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
Quoted 39 lines · On Thu, 14 May 2026 02:01:45 +0000 [email protected] 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 <[email protected]>
Subject:mm/damon/lru_sort: detect and use fresh enabled andkdamond_pidvaluesThis commit updates the
DAMON_LRU_SORTmodule to dynamically detect and report the fresh status of enabled andkdamond_pidparameters. 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 callingdamon_is_running()anddamon_kdamond_pid(), the system aims to provide accurate runtime information to sysfs readers.Quoted 4 lines
mm/damon/lru_sort.c +0
-0[ ... ]
mm/damon/lru_sort.c static DECLARE_DELAYED_WORK(damon_lru_sort_timer, damon_lru_sort_timer_fn); · line 276
Quoted 10 lines
mm/damon/lru_sort.c +6
-0@@ -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?
Quoted 2 lines
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.
Quoted 10 lines
it calls
damon_destroy_ctx(ctx)to free the context. However, the global static ctx pointer is not set to NULL.Since
DAMON_LRU_SORTis 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 todamon_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.
Quoted 32 lines
[ ... ]
Quoted 5 lines
mm/damon/lru_sort.c +1
-0@@ -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,mm/damon/lru_sort.c static int damon_lru_sort_enabled_store(const char *val, · line 303
Quoted 10 lines
mm/damon/lru_sort.c +6
-1+ 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_enabledvariables 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 becauselast_enabled==now_enabled.If the user tries to reset the state by writing 'N',
damon_lru_sort_turn(false)callsdamon_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:
Quoted 7 lines
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
Quoted 3 lines
Signature · 2 lines
-- Sashiko AI review · https://sashiko.dev/#/patchset/2026051243-crevice-spool-75d8@gregkh?part=2
SeongJae Park
maintainerRe: [PATCH 6.1.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
In reply to SeongJae ParkOn Tue, 12 May 2026 21:30:36 -0700 SeongJae Park <[email protected]> wrote:
[...]
Quoted 9 lines
Fixes 40e983cca927 ("mm/damon: introduce DAMON-based LRU-lists Sorting")Co-developed-by Liew Rui Yan <[email protected]>Signed-off-by Liew Rui Yan <[email protected]>Signed-off-by SeongJae Park <[email protected]>Cc <[email protected]> # 6.0.xSigned-off-by Andrew Morton <[email protected]>(cherry picked from commit b98b7ff6025ae82570d4915e083f0cbd8d48b3cf)Signed-off-by SeongJae Park <[email protected]>
Quoted 3 lines
(port parts of
42b7491af14c("mm/damon/core: introducedamon_call()") andd2b5be741a50("mm/damon/sysfs: use DAMON core APIdamon_is_running()") fordamon_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/[email protected]
Thanks,
SJ
[...]
SeongJae Park
maintainerPATCH v2 2/2 mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values
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 kdamondThe 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.
(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)
---
Changes from v1
- v1: https://lore.kernel.org/[email protected]
- Fixup incomplete enabled_store() part porting
include/linux/damon.h | 1 + mm/damon/core.c | 16 +++++++++ mm/damon/lru_sort.c | 84 ++++++++++++++++++++++++++++--------------- 3 files changed, 73 insertions(+), 28 deletions(-)
include/linux/damon.h +1 -0
@@ -572,6 +572,7 @@ static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrsint 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,
mm/damon/core.c +16 -0
@@ -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.
mm/damon/lru_sort.c +56 -28
123 lines not on this page · raw message
Signature · 1 line
2.47.3