Re: [PATCH v7 11/12] virt/steal_monitor: Act on steal time periodically and decide on preferred CPUs

[Yury Norov <ynorov@nvidia.com>]

On Fri, Jul 10, 2026 at 03:26:47AM +0530, Shrikanth Hegde wrote:
> schedule work at regular intervals. Interval is determined by
> interval_ms parameter. schedule_delayed_work is used since interval_ms
> is usually in order of milliseconds. Work need not happen instantly.
> 
> Periodic work function essentially does:
> - Calculate the steal_ratio as below.
> 
>        steal_ratio = (delta_steal * 100*100)/(delta_ns * num_cpus())
> 
>   It is calculated to consider the fractional values of steal time.
>   I.e 10 means 0.1% steal time. A few tricks such as divide by 10,000
>   are used to avoid possible overflow.
> - 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.
> - Save the current direction of steal values to avoid oscillations.
>   So two consecutive values of high values or low values are taken for
>   decrease/increase of preferred CPUs.
> - Ensure design checks are met.
>   1. At least one core/CPU must be there in preferred mask.
>   2. preferred CPUs is subset of active CPUs.
> 
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
> v6->v7:
> - Merge two patches which did periodic work function.
> - Misc checks for early firing, requeue work, math safety.
> 
>  drivers/virt/steal_monitor/sm_core.c | 76 +++++++++++++++++++++++++++-
>  drivers/virt/steal_monitor/sm_core.h |  1 +
>  2 files changed, 76 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virt/steal_monitor/sm_core.c b/drivers/virt/steal_monitor/sm_core.c
> index 4a03c14337be..09a5c3a299c3 100644
> --- a/drivers/virt/steal_monitor/sm_core.c
> +++ b/drivers/virt/steal_monitor/sm_core.c
> @@ -20,6 +20,12 @@ struct steal_monitor sm_core_ctx = {
>  	.low_threshold = 200,	/* 2% */
>  };
>  
> +enum sm_direction {
> +	SM_DIR_INCREASE = -1,
> +	SM_DIR_NONE	=  0,
> +	SM_DIR_DECREASE	=  1,
> +};
> +
>  static int param_set_interval_ms(const char *val, const struct kernel_param *kp)
>  {
>  	unsigned int interval;
> @@ -106,14 +112,82 @@ module_param_cb(low_threshold, &low_threshold_ops, &sm_core_ctx.low_threshold, 0
>  MODULE_PARM_DESC(low_threshold,
>  		 "Low steal threshold. default: 200 i.e 2%. Must be < high_threshold");
>  
> +static void compute_preferred_cpus_work(struct work_struct *work)
> +{
> +	u64 curr_steal, delta_steal, delta_ns, steal_ratio;
> +	ktime_t now;
> +
> +	now = ktime_get();
> +	delta_ns = ktime_to_ns(ktime_sub(now, sm_core_ctx.prev_time));
> +
> +	if (unlikely(delta_ns < NSEC_PER_MSEC)) {
> +		pr_err_ratelimited("steal_monitor: work scheduled too soon delta_ns: %llu\n",
> +				   delta_ns);
> +		goto requeue_work;
> +	}
> +
> +	curr_steal = get_system_steal_time();
> +	delta_steal = curr_steal > sm_core_ctx.prev_steal ?
> +		      curr_steal - sm_core_ctx.prev_steal : 0;
> +
> +	/* Update for next calculation */
> +	sm_core_ctx.prev_steal = curr_steal;
> +	sm_core_ctx.prev_time = now;
> +
> +	/*
> +	 * steal_ratio = (delta_steal * 100*100)/(delta_ns * num_cpus())
> +	 * To avoid possible overflow, divide the denominator early.
> +	 * Note minimum interval is 10ms.
> +	 */
> +	delta_ns = div_u64(delta_ns * get_num_cpus_steal_ratio(), 100 * 100);
> +	steal_ratio = div64_u64(delta_steal, delta_ns);
> +
> +	if (sm_core_ctx.prev_direction == SM_DIR_DECREASE &&
> +	    steal_ratio > sm_core_ctx.high_threshold)
> +		decrease_preferred_cpus(&sm_core_ctx);
> +	if (sm_core_ctx.prev_direction == SM_DIR_INCREASE &&
> +	    steal_ratio <= sm_core_ctx.low_threshold)
> +		increase_preferred_cpus(&sm_core_ctx);

I already said, I don't like this SM_DIR approach. If you want to
avoid oscillations, just increase the gap. If it doesn't work, then we
need to understand why.

> +
> +	/*
> +	 * mark the direction. Increasing the gap between hi and lo_threshold
> +	 * helps to avoid ping-pongs.
> +	 */
> +	if (steal_ratio > sm_core_ctx.high_threshold)
> +		sm_core_ctx.prev_direction = SM_DIR_DECREASE;
> +	else if (steal_ratio <= sm_core_ctx.low_threshold)
> +		sm_core_ctx.prev_direction = SM_DIR_INCREASE;
> +	else
> +		sm_core_ctx.prev_direction = SM_DIR_NONE;
> +
> +requeue_work:
> +	/* maintain design constructs always */
> +	WARN_ON_ONCE(cpumask_empty(cpu_preferred_mask));
> +	WARN_ON_ONCE(!cpumask_subset(cpu_preferred_mask, cpu_active_mask));

cpu_read_lock here? And again, you should do something to restore
integrity. WARN_ON is not enough. The simplest and safest thing you
can do is to unload the driver. You definitely shouldn't schedule a
new work against the broken cpu_preferred_mask.

> +
> +	/* Trigger for next sampling */
> +	schedule_delayed_work(&sm_core_ctx.work,
> +			      msecs_to_jiffies(sm_core_ctx.interval_ms));
> +}
> +
>  static int __init steal_monitor_init(void)
>  {
> -	pr_info("steal_monitor is enabled\n");
> +	pr_info("steal_monitor is enabled. interval: %ums, high_threshold: %u, low_threshold: %u\n",
> +		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));
> +
>  	return 0;
>  }
>  
>  static void __exit steal_monitor_exit(void)
>  {
> +	cancel_delayed_work_sync(&sm_core_ctx.work);

cancel_delayed_work_sync() is not enough for a self-requeueing work.
compute_preferred_cpus_work() always requeues itself. Module unload
can return with delayed work armed against module text/data. Use
disable_delayed_work_sync() or a stop flag checked before requeueing.

>  	guard(cpus_read_lock)();
>  	cpumask_copy(&__cpu_preferred_mask, cpu_active_mask);
>  
> diff --git a/drivers/virt/steal_monitor/sm_core.h b/drivers/virt/steal_monitor/sm_core.h
> index ee68cd8b1944..7c7a9bced682 100644
> --- a/drivers/virt/steal_monitor/sm_core.h
> +++ b/drivers/virt/steal_monitor/sm_core.h
> @@ -14,6 +14,7 @@
>  #include <linux/kernel_stat.h>
>  #include <linux/topology.h>
>  #include <linux/sched/isolation.h>
> +#include <linux/math64.h>
>  
>  struct steal_monitor {
>  	struct delayed_work	work;
> -- 
> 2.47.3