From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A4E7639D6FD; Wed, 13 May 2026 05:05:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778648712; cv=none; b=kVmhvPFxKUOpxqF2ejNWxJhv/Y+FuDWebR5cNjfLfN/datvG86d+mQ3I2yz3rZg95rFQNVUdnID+sgeihEz/9FJufxHLO+xOV9K7usUJcJtQ4DrhCEEBm6wX9/CmR2z7BjoAlhjVhLFZ8yftffeLzxrTsKCw7iB3Ocq2URStts0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778648712; c=relaxed/simple; bh=KvXZQmW0LsqEWFm1C1I47QuY02g2RND7AXmxV2MoG3Q=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=sI1ueb8mDq2aOExLeElJONOnGPw8c1CgIyIX5LUZGBCriedJp73/73kALfNXFJADwDAXcyMRSR1vPkXB4fAw2rOM7ZafGB5pL7hZ86qAgJxt+7mphm7y19SIJzIraJFPcazq3L00Hc1OQJ3XAnK23nPou03ZY5QGzD+aoSxyppY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=S6aI71sV; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="S6aI71sV" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B0EFFC2BCC7; Wed, 13 May 2026 05:05:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1778648712; bh=KvXZQmW0LsqEWFm1C1I47QuY02g2RND7AXmxV2MoG3Q=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=S6aI71sVJrLLhYG5jrRdKt77hqTe8ATtMsWccAk545NB/VSNudKeML+jyrnN6CAGm RA90pr8DZrheW9WJWlJWWRL0KcUn2wZVybO39ZOT3vBqxcbqp06LgNI8X7iynIPAIT FIO8olz+OpQk6MP3h0eoHXm/Go0WuxtAslwzkGqkxIrZ31s/9TuVSyIFTUUSmcO3zG CP0/ln83ig7ivJd+QchDlNUK9CmO623Dpmxp2f1siqkdjShboPtAMH7ZuU+zKKt2Qa rE25gg6/77ZANAfMHxEypuij5zXUDJZyfIJWYHOnoCrIOoJjsEmIXA+jIAHRCUt3ra B2u8G5Hwb3FmQ== From: SeongJae Park To: stable@vger.kernel.org Cc: SeongJae Park , damon@lists.linux.dev, Liew Rui Yan , Andrew Morton Subject: [PATCH 6.1.y] mm/damon/reclaim: detect and use fresh enabled and kdamond_pid values Date: Tue, 12 May 2026 22:05:00 -0700 Message-ID: <20260513050501.216835-1-sj@kernel.org> X-Mailer: git-send-email 2.47.3 In-Reply-To: <2026051257-daytime-precise-afd3@gregkh> References: <2026051257-daytime-precise-afd3@gregkh> Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Patch series "mm/damon/modules: detect and use fresh status", v3. DAMON modules including DAMON_RECLAIM, DAMON_LRU_SORT and DAMON_STAT commonly expose the kdamond running status via their parameters. Under certain scenarios including wrong user inputs and memory allocation failures, those parameter values can be stale. It can confuse users. For DAMON_RECLAIM and DAMON_LRU_SORT, it even makes the kdamond unable to be restarted before the system reboot. The problem comes from the fact that there are multiple events for the status changes and it is difficult to follow up all the scenarios. Fix the issue by detecting and using the status on demand, instead of using a cached status that is difficult to be updated. Patches 1-3 fix the bugs in DAMON_RECLAIM, DAMON_LRU_SORT and DAMON_STAT in the order. This patch (of 3): DAMON_RECLAIM 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_RECLAIM 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_reclaim/parameters # # # start DAMON_RECLAIM # 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-1-sj@kernel.org Link: https://lore.kernel.org/20260419161003.79176-2-sj@kernel.org Fixes: e035c280f6df ("mm/damon/reclaim: support online inputs update") Co-developed-by: Liew Rui Yan Signed-off-by: Liew Rui Yan Signed-off-by: SeongJae Park Cc: # 5.19.x Signed-off-by: Andrew Morton (cherry picked from commit 64a140afa5ed1c6f5ba6d451512cbdbbab1ba339) Signed-off-by: SeongJae Park --- This depends on other two backported patches [1,2]. Please apply this after those. [1] https://lore.kernel.org/20260513043039.173237-1-sj@kernel.org [2] https://lore.kernel.org/20260513043039.173237-2-sj@kernel.org mm/damon/reclaim.c | 74 +++++++++++++++++++++++++++++++++------------- 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c index 7952a0b7f409d..dc435aefd44f5 100644 --- a/mm/damon/reclaim.c +++ b/mm/damon/reclaim.c @@ -100,15 +100,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_RECLAIM 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_reclaim_stat; DEFINE_DAMON_MODULES_DAMOS_STATS_PARAMS(damon_reclaim_stat, reclaim_tried_regions, reclaimed_regions, quota_exceeds); @@ -184,22 +175,14 @@ static int damon_reclaim_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_reclaim_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_reclaim_timer; @@ -220,6 +203,13 @@ static DECLARE_DELAYED_WORK(damon_reclaim_timer, damon_reclaim_timer_fn); static bool damon_reclaim_initialized; +static bool damon_reclaim_enabled(void) +{ + if (!ctx) + return false; + return damon_is_running(ctx); +} + static int damon_reclaim_enabled_store(const char *val, const struct kernel_param *kp) { @@ -236,15 +226,57 @@ static int damon_reclaim_enabled_store(const char *val, return 0; } +static int damon_reclaim_enabled_load(char *buffer, + const struct kernel_param *kp) +{ + return sprintf(buffer, "%c\n", damon_reclaim_enabled() ? 'Y' : 'N'); +} + static const struct kernel_param_ops enabled_param_ops = { .set = damon_reclaim_enabled_store, - .get = param_get_bool, + .get = damon_reclaim_enabled_load, }; module_param_cb(enabled, &enabled_param_ops, &enabled, 0600); MODULE_PARM_DESC(enabled, "Enable or disable DAMON_RECLAIM (default: disabled)"); +static int damon_reclaim_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_reclaim_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_reclaim_kdamond_pid_store, + .get = damon_reclaim_kdamond_pid_load, +}; + +/* + * PID of the DAMON thread + * + * If DAMON_RECLAIM 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_reclaim_handle_commit_inputs(void) { int err; -- 2.47.3