mm/damon/stat: detect and use fresh enabled value

DAMON_STAT updates 'enabled' parameter value, which represents the running
status of its kdamond, when the user explicitly requests start/stop of the
kdamond.  The kdamond can, however, be stopped even if the user explicitly
requested the stop, if ctx->regions_score_histogram allocation failure at
beginning of the execution of the kdamond.  Hence, if the kdamond is
stopped by the allocation failure, the value of the parameter can be
stale.

Users could show the stale value and be confused.  The problem will only
rarely happen in real and common setups because the allocation is arguably
too small to fail.  Also, unlike the similar bugs that are now fixed in
DAMON_RECLAIM and DAMON_LRU_SORT, kdamond can be restarted in this case,
because DAMON_STAT force-updates the enabled parameter value for user
inputs.  The bug is a bug, though.

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.

The issue was dicovered [1] by Sashiko.

Link: https://lore.kernel.org/20260419161003.79176-4-sj@kernel.org
Link: https://lore.kernel.org/20260416040602.88665-1-sj@kernel.org
Fixes: 369c415e6073 ("mm/damon: introduce DAMON_STAT module")
Signed-off-by: SeongJae Park <sj@kernel.org>
Cc: Liew Rui Yan <aethernet65535@gmail.com>
Cc: <stable@vger.kernel.org> # 6.17.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index 99ba346f9e32576b7de7f004ff8bad8bbc3d08dd..3951b762cbddf6f89fc9393fcbb3283344d21e39 100644 (file)
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -19,14 +19,17 @@
 static int damon_stat_enabled_store(
                const char *val, const struct kernel_param *kp);
 
+static int damon_stat_enabled_load(char *buffer,
+               const struct kernel_param *kp);
+
 static const struct kernel_param_ops enabled_param_ops = {
        .set = damon_stat_enabled_store,
-       .get = param_get_bool,
+       .get = damon_stat_enabled_load,
 };
 
 static bool enabled __read_mostly = IS_ENABLED(
        CONFIG_DAMON_STAT_ENABLED_DEFAULT);
-module_param_cb(enabled, &enabled_param_ops, &enabled, 0600);
+module_param_cb(enabled, &enabled_param_ops, NULL, 0600);
 MODULE_PARM_DESC(enabled, "Enable of disable DAMON_STAT");
 
 static unsigned long estimated_memory_bandwidth __read_mostly;
@@ -273,17 +276,23 @@ static void damon_stat_stop(void)
        damon_stat_context = NULL;
 }
 
+static bool damon_stat_enabled(void)
+{
+       if (!damon_stat_context)
+               return false;
+       return damon_is_running(damon_stat_context);
+}
+
 static int damon_stat_enabled_store(
                const char *val, const struct kernel_param *kp)
 {
-       bool is_enabled = enabled;
        int err;
 
        err = kstrtobool(val, &enabled);
        if (err)
                return err;
 
-       if (is_enabled == enabled)
+       if (damon_stat_enabled() == enabled)
                return 0;
 
        if (!damon_initialized())
@@ -293,16 +302,17 @@ static int damon_stat_enabled_store(
                 */
                return 0;
 
-       if (enabled) {
-               err = damon_stat_start();
-               if (err)
-                       enabled = false;
-               return err;
-       }
+       if (enabled)
+               return damon_stat_start();
        damon_stat_stop();
        return 0;
 }
 
+static int damon_stat_enabled_load(char *buffer, const struct kernel_param *kp)
+{
+       return sprintf(buffer, "%c\n", damon_stat_enabled() ? 'Y' : 'N');
+}
+
 static int __init damon_stat_init(void)
 {
        int err = 0;