Hello Nicheng,
On Thu, 14 May 2026 15:48:46 +0800 niecheng <niecheng1@uniontech.com> wrote:
> damon_set_attrs() refreshes next_aggregation_sis and
> next_ops_update_sis for online monitoring attribute updates, but it
> does not refresh next_intervals_tune_sis.
>
> Because of that, enabling intervals auto-tuning via an online attrs
> commit can leave next_intervals_tune_sis stale. If a context starts
> with intervals_goal.aggrs == 0 and later updates attrs online to set it
> non-zero, kdamond_fn() can treat the tuning deadline as already expired
> and tune the intervals earlier than intended.
>
> This has been possible since the intervals auto-tuning feature was
> introduced, because that commit initialized the deadline at kdamond
> start but did not refresh it on later attrs updates.
Good finding, thank you for sharing this!
But, the next_interval_tune_sis will be updated based on the updated
aggregation and sampling intervals, just before the next intervals tuning is
made.
In detail, the code flow of the kdamond_fn() main loop is like below:
- call kdamond_call()
- call damon_set_attrs()
- update aggregation and sampling intervals
- if passed_ample_intervals >= next_intervals_tune_sis:
- update next_intervals_tune_sis with updated aggregation and sampling
intervals
- call kdamond_tune_intervals()
So, the old next_intervals_tune_sis will be used only once. I agree not
everyone will think it is the best behavior. But seems ok to me. I'd like to
keep the current code in favor of less complexity. What do you think?
Nevertheless, apparently the code can better be documented. Maybe it is worthy
to add a comment about this. For example, maybe it is better to add a comment
saying "next_intervals_tune_sis will be updated inside kdamond_fn()" on the
damon_set_attrs(). If you'd like to, please feel free to post such a patch.
Thanks,
SJ
[...]