Re: [RFC PATCH v2 1/3] mm/damon/reclaim: detect and use fresh enabled and kdamond_pid values

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Medium] Command-line override for the 'enabled' parameter is broken: passing 'enabled=N' after 'enabled=Y' fails to disable the module.
--

commit 6f46639cc34c6ddcbecc0aba915988dff8f651de
Author: SeongJae Park <sj@kernel.org>

mm/damon/reclaim: detect and use fresh enabled and kdamond_pid values

This commit updates DAMON_RECLAIM to dynamically query the running status
of kdamond instead of relying on cached parameter values. This ensures
that when kdamond stops unexpectedly, the enabled and kdamond_pid parameters
reflect the correct state.

> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 86da147786583..46660c8d4d422 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c

[ ... ]

> @@ -332,10 +316,16 @@ module_param_cb(addr_unit, &addr_unit_param_ops, &addr_unit, 0600);
>  MODULE_PARM_DESC(addr_unit,
>  	"Scale factor for DAMON_RECLAIM to ops address conversion (default: 1)");
>  
> +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)
>  {
> -	bool is_enabled = enabled;
>  	bool enable;
>  	int err;
>  
> @@ -343,7 +333,7 @@ static int damon_reclaim_enabled_store(const char *val,
>  	if (err)
>  		return err;
>  
> -	if (is_enabled == enable)
> +	if (damon_reclaim_enabled() == enable)
>  		return 0;

Does changing this check break kernel command-line overrides during boot?

During early boot when command-line parameters are parsed, ctx is not yet
allocated, so damon_reclaim_enabled() unconditionally returns false.

If a system has damon_reclaim.enabled=Y in the boot parameters and an
administrator appends damon_reclaim.enabled=N to override it, the first
evaluation correctly sets the internal enabled flag to true.

However, when the second parameter (N) is processed, enable is false.
Since damon_reclaim_enabled() also returns false, the condition matches
and the function returns 0 immediately. The internal enabled variable is
never updated to false, causing the module to start unexpectedly when
damon_reclaim_init() runs.

Should this keep checking the internal enabled variable during early boot?

>  
>  	/* Called before init function.  The function will handle this. */
> @@ -359,15 +349,46 @@ static int damon_reclaim_enabled_store(const char *val,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260418014439.6353-1-sj@kernel.org?part=1