Re: [PATCH v6 16/23] virt/steal_monitor: Compute work at regular intervals

[Yury Norov <ynorov@nvidia.com>]

On Wed, Jul 01, 2026 at 07:46:47PM +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@linux.ibm.com>
> ---
>  drivers/virt/steal_monitor/sm_core.c | 26 +++++++++++++++++++++++++-
>  drivers/virt/steal_monitor/sm_core.h |  3 +++
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virt/steal_monitor/sm_core.c b/drivers/virt/steal_monitor/sm_core.c
> index 1ba638224abb..b499faa61010 100644
> --- a/drivers/virt/steal_monitor/sm_core.c
> +++ b/drivers/virt/steal_monitor/sm_core.c
> @@ -32,9 +32,32 @@ module_param_named(low_threshold, sm_core_ctx.low_threshold, uint, 0644);
>  MODULE_PARM_DESC(low_threshold,
>  		 "Low steal threshold (default: 200 i.e 2%)");
>  
> +static void compute_preferred_cpus_work(struct work_struct *work)
> +{
> +	/* At least one core is kept as preferred */
> +	WARN_ON(cpumask_empty(cpu_preferred_mask));

This is very true, at least one CPU must be preferred. But throwing
warnings is useless. What do you want me to do if I see this warning?
The only possible solution I see is unloading the driver and not using
this balancer at all. Don't think it's what you want me to do.

Your logic should make it impossible to have the preferred cpumask
empty.

> +
> +	/* Warn if interval_ms is set to 0, that might cause lockup. */
> +	if (unlikely(sm_core_ctx.interval_ms == 0)) {
> +		WARN_ON(1);
> +		sm_core_ctx.interval_ms = 1000; /* Fallback to default */
> +	}

WARN_ON() means panic under some configurations. You shouldn't do
that. The proper way of handling it is failing in steal_monitor_init().
This function is int, not a void, for a reason.

> +	/* 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);
> +
> +	schedule_delayed_work(&sm_core_ctx.work,
> +			      msecs_to_jiffies(sm_core_ctx.interval_ms));
> +
>  	return 0;
>  }
>  
> @@ -42,6 +65,7 @@ static void __exit steal_monitor_exit(void)
>  {
>  	pr_info("steal_monitor is disabled\n");
>  
> +	cancel_delayed_work_sync(&sm_core_ctx.work);
>  	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 e5c3ea0a63c9..ea06e83c228c 100644
> --- a/drivers/virt/steal_monitor/sm_core.h
> +++ b/drivers/virt/steal_monitor/sm_core.h
> @@ -8,6 +8,9 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/cpuhplock.h>
> +#include <linux/cpumask.h>
> +#include <linux/workqueue.h>
> +#include <linux/sched/isolation.h>
>  
>  struct steal_monitor {
>  	struct delayed_work	work;
> -- 
> 2.47.3