Re: [PATCH v6 20/23] virt/steal_monitor: Act on steal values at regular intervals

[Yury Norov]

On Wed, Jul 01, 2026 at 07:46:51PM +0530, Shrikanth Hegde wrote:
> This is the steal_monitor core functionality done in periodic work
> 
> - Calculate the steal_ratio. It is multiplied by 100 to consider the
>   fractional values of steal time. I.e 10 means 0.1% steal time.
> - If steal value is higher than high threshold, call the method to reduce
>   the preferred CPUs.
> - If steal value is lower or equal to low threshold, call the method to
>   increase the preferred CPUs.
> - If the steal value is in between, no action is taken.
> - Save the values for next delta calculations.
> 
> Signed-off-by: Shrikanth Hegde <sshegde@xxxxxxxxxxxxx>
> ---
> v5->v6:
> - Address u64 overflow concerns.
> 
>  drivers/virt/steal_monitor/sm_core.c | 33 ++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/virt/steal_monitor/sm_core.c b/drivers/virt/steal_monitor/sm_core.c
> index b499faa61010..7b7435f79b85 100644
> --- a/drivers/virt/steal_monitor/sm_core.c
> +++ b/drivers/virt/steal_monitor/sm_core.c
> @@ -34,6 +34,37 @@ MODULE_PARM_DESC(low_threshold,
>  
>  static void compute_preferred_cpus_work(struct work_struct *work)
>  {
> +	u64 curr_steal, delta_steal, delta_ns, steal_ratio;
> +	ktime_t now;
> +
> +	curr_steal = get_system_steal_time();
> +	now = ktime_get();
> +
> +	/* get the deltas */
> +	delta_steal = curr_steal > sm_core_ctx.prev_steal ?
> +		      curr_steal - sm_core_ctx.prev_steal : 0;
> +	delta_ns = max_t(u64, ktime_to_ns(ktime_sub(now, sm_core_ctx.prev_time)), 1);

The below return on '!delta_ns' makes this max(...) useless, right?
Regardless, if the time between 2 measures is less then 1ns, I
believe, the whole measure is not trustworthy

> +
> +	/* Update for next calculation */
> +	sm_core_ctx.prev_steal = curr_steal;
> +	sm_core_ctx.prev_time = now;

So below return should go prior to this update, because 'now' is actually
the same as 'prev_time', right?

I don't understand why 'now' can be so close to prev_time, because
you've scheduled this callback on the regular interval. But if that's
possible, can you explain that and do like this at the very beginning
of the function:

        now = ktime_get();
        if (unlikely(now < sm_core_ctx.prev_time + sm_core_ctx.interval / 2)) {
                pr_warn(...);
                return;
        }

And if it's a never-happen condition, just use WARN_ON().

> +
> +	/*
> +	 * Multiply by 100 to consider the fractional values of steal time.
> +	 * steal_ratio = (delta_steal * 100 * 100)/(delta_ns * num_cpus())
> +	 */
> +	delta_ns = div_u64(delta_ns * get_num_cpus_steal_ratio(), 100 * 100);

You're not multiplying by 100, you're dividing by 10k. Can you reword the
comment?

> +	if (unlikely(!delta_ns))
> +		return;
> +
> +	steal_ratio = div64_u64(delta_steal, delta_ns);
> +	/* If the steal time values are high, reduce preferred CPUs */

I really believe that the below code is clear enough, worth nothing
explaining it.

> +	if (steal_ratio > sm_core_ctx.high_threshold)
> +		decrease_preferred_cpus(&sm_core_ctx);
> +	/* If the steal time values are low, increase preferred CPUs */
> +	if (steal_ratio <= sm_core_ctx.low_threshold)
> +		increase_preferred_cpus(&sm_core_ctx);
> +
>  	/* At least one core is kept as preferred */
>  	WARN_ON(cpumask_empty(cpu_preferred_mask));
>  
> @@ -54,6 +85,8 @@ static int __init steal_monitor_init(void)
>  		sm_core_ctx.interval_ms, sm_core_ctx.high_threshold, sm_core_ctx.low_threshold);
>  
>  	INIT_DELAYED_WORK(&sm_core_ctx.work, compute_preferred_cpus_work);
> +	sm_core_ctx.prev_steal = get_system_steal_time();
> +	sm_core_ctx.prev_time = ktime_get();
>  
>  	schedule_delayed_work(&sm_core_ctx.work,
>  			      msecs_to_jiffies(sm_core_ctx.interval_ms));
> -- 
> 2.47.3