Re: [PATCH 4/4] mm/damon/sysfs: set next refresh jiffies per sysfs context

[Kunwu Chan]

On Tue,  1 Sep 2026 17:27:23 -0700 SJ Park <sj@xxxxxxxxxx> wrote:

> When 'refresh_ms' is set, DAMON sysfs interface periodically updates
> auto-tuned parameters and DAMOS stats.  The timestamp for the next
> refresh is initialized when a DAMON context starts, and updated in its
> damon_call() callback function.  That is, each DAMON context updates it.
> However, the timestamp is a global variable that is shared with all the
> contexts.  When there are multiple DAMON contexts having different
> refresh_ms, the update frequency will be changed, depending on the order
> of the contexts.  When there are multiple kdamonds, it will be even more
> chaotic.  Fix the problem by having the timestamp per each context.
> 
> The user impact is not very critical.  It does not leak, corrupt or
> crash.  The update will not be faster or slower than the lowest and
> largest refresh_ms values of the contexts, respectively.  The user can
> also manually ask the updates on demand using kdamond state commands.
> That said, clearly this is a bug and can easily be reproduced.
> 
> Fixes: 9fd7bb5083d1 ("mm/damon/sysfs: change next_update_jiffies to a global variable")
> Cc: <stable@xxxxxxxxxxxxxxx> # 6.18.x
> Signed-off-by: SJ Park <sj@xxxxxxxxxx>
> ---
>  mm/damon/sysfs.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index 3c81b4c91ac0d..dcb739ce0a729 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -1788,6 +1788,7 @@ struct damon_sysfs_kdamond {
>  	struct damon_sysfs_contexts *contexts;
>  	struct damon_ctx *damon_ctx;
>  	unsigned int refresh_ms;
> +	unsigned long next_refresh_jiffies;
>  };
>  
>  static struct damon_sysfs_kdamond *damon_sysfs_kdamond_alloc(void)
> @@ -2213,17 +2214,15 @@ static struct damon_ctx *damon_sysfs_build_ctx(
>  	return ctx;
>  }
>  
> -static unsigned long damon_sysfs_next_update_jiffies;
> -
>  static int damon_sysfs_repeat_call_fn(void *data)
>  {
>  	struct damon_sysfs_kdamond *sysfs_kdamond = data;
>  
>  	if (!sysfs_kdamond->refresh_ms)
>  		return 0;
> -	if (time_before(jiffies, damon_sysfs_next_update_jiffies))
> +	if (time_before(jiffies, sysfs_kdamond->next_refresh_jiffies))
>  		return 0;
> -	damon_sysfs_next_update_jiffies = jiffies +
> +	sysfs_kdamond->next_refresh_jiffies = jiffies +
>  		msecs_to_jiffies(sysfs_kdamond->refresh_ms);
>  
>  	if (!mutex_trylock(&damon_sysfs_lock))
> @@ -2271,8 +2270,8 @@ static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
>  	}
>  	kdamond->damon_ctx = ctx;
>  
> -	damon_sysfs_next_update_jiffies =
> -		jiffies + msecs_to_jiffies(kdamond->refresh_ms);
> +	kdamond->next_refresh_jiffies = jiffies +
> +		msecs_to_jiffies(kdamond->refresh_ms);

The fix looks correct to me.

Moving next_refresh_jiffies into struct damon_sysfs_kdamond gives 
each kdamond ownership of its refresh schedule, so refresh activity 
from one kdamond can no longer interfere with another.

I also checked the initialization path: the new field is zero-initialized 
by kzalloc_obj(), and damon_sysfs_turn_damon_on() sets it to 
jiffies + refresh_ms before registering the callback via damon_call().

Reviewed-by: Kunwu Chan <kunwu.chan@xxxxxxxxx>

Thanks,
Kunwu

>  
>  	repeat_call_control->fn = damon_sysfs_repeat_call_fn;
>  	repeat_call_control->data = kdamond;
> -- 
> 2.47.3
> 

Sent using hkml (https://github.com/sjp38/hackermail)