Re: [PATCH v9 10/11] virt/steal_governor: Implement steal_governor policy loop

[Yury Norov <ynorov@nvidia.com>]

On Fri, Jul 24, 2026 at 07:37:31PM +0530, Shrikanth Hegde wrote:
> schedule work at regular intervals to implement the steal_governor
> policy of monitoring the steal time and take action on the state of
> preferred CPUs. The interval is determined by interval_ms parameter.
> schedule_delayed_work is used since interval_ms
> is in the order of milliseconds. Work need not happen instantly.
> 
> Periodic policy loop essentially does:
> - Gets the total/delta steal values and cpus to use steal_ratio.
> - 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.
> - 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.
> If not met, then restore preferred CPUs to active and stop
> requeue of the work. Driver is effectively non-functional after that.
> 
> 
> In order to help the above loop, a few helper functions have been added.
> 
> 1. get_system_steal_time()
> - steal governor takes global view of steal time instead of individual
>   vCPU. Collect the steal values across the vCPUs of interest.
> - Sum up steal time values across possible CPUs. This helps to keep it
>   a monotonically increasing number and avoids spikes due to CPU
>   hotplug.

This is questionable. See below. There should be a better way to
calculate steal ratio, rather then accounting offlined CPUs stats.

> 
> 2. decrease_preferred_cpus()
> - Called when there is high steal time. It needs to decide which CPUs to
>   mark as non-preferred and set that state.
> - Get first housekeeping CPU and its core mask. Mark it as
>   protected core. This helps to keep at least one core as preferred.
>   kernel ensures at least one housekeeping CPU stays active.
> - Find the last CPU outside of this protected core mask. (target CPU)
> - Based on that target CPU, get its sibling and mark them as
>   non-preferred.
> 
> 3. increase_preferred_cpus()
> - Called when there is low steal time. It needs to decide which CPUs to
>   mark as preferred and set that state.
> - Get the first active non-preferred CPUs. This likely is the last
>   set of CPUs being marked as non-preferred.
> - get the siblings of that CPU and mark them as preferred.
> 
> 4. get_system_cpus()
> - informs how many CPUs needs to be considered for steal_ratio
>   calculations.
> - Return number of possible CPUs as get_system_steal_time computes
>   steal values across possible CPUs.
> 
> Notes:
> 1. Using core instead of individual CPUs performs better as SMT is
>    quite common and some hypervisor such as powerVM does core scheduling.
> 
> 2. This doesn't do any NUMA splicing to keep the code simpler and
>    minimal overhead. Current code expects CPUs spread uniformly
>    across NUMA nodes.
> 
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
>  drivers/virt/steal_governor.c | 155 ++++++++++++++++++++++++++++++++++
>  1 file changed, 155 insertions(+)
> 
> diff --git a/drivers/virt/steal_governor.c b/drivers/virt/steal_governor.c
> index fda86777d6f0..991821a82485 100644
> --- a/drivers/virt/steal_governor.c
> +++ b/drivers/virt/steal_governor.c
> @@ -13,13 +13,18 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <linux/cleanup.h>
>  #include <linux/cpuhplock.h>
>  #include <linux/cpumask.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
> +#include <linux/kernel_stat.h>
>  #include <linux/kconfig.h>
>  #include <linux/ktime.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
> +#include <linux/sched/isolation.h>
> +#include <linux/topology.h>
>  #include <linux/types.h>
>  #include <linux/workqueue.h>
>  
> @@ -108,6 +113,151 @@ module_param_named(low_threshold, sg_ctx.low_threshold, uint, 0444);
>  MODULE_PARM_DESC(low_threshold,
>  		 "Low steal threshold. default: 200 i.e 2%. Must be < high_threshold");
>  
> +/* Return collective steal time across system. */
> +static u64 get_system_steal_time(void)
> +{
> +	int cpu;
> +	u64 total_steal = 0;
> +
> +	for_each_possible_cpu(cpu)
> +		total_steal += kcpustat_cpu(cpu).cpustat[CPUTIME_STEAL];
> +
> +	return total_steal;
> +}

In v8 I pointed to the identical function in s390 code, and you agreed
to unify them, but that didn't happen. Please do that in the next
version.

> +/* Return number of CPUs to consider steal ratio. */
> +static unsigned int get_system_cpus(void)
> +{
> +	return num_possible_cpus();
> +}
> +
> +/*
> + *

Useless line

> + * Called when the steal governor detects high physical CPU contention.
> + * It finds the last active core in the preferred mask and mark those
> + * CPUs as non-preferred.
> + *
> + * Must ensure:
> + * - at least one core is always kept as preferred
> + * - preferred is always subset of active.
> + */
> +static void decrease_preferred_cpus(void)
> +{
> +	const struct cpumask *first_hk_core;
> +	int target_cpu = nr_cpu_ids;
> +	int cpu;
> +
> +	guard(cpus_read_lock)();
> +	cpu = cpumask_first_and(housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
> +				cpu_preferred_mask);
> +	if (cpu >= nr_cpu_ids)
> +		return;
> +
> +	/* Always leave first housekeeping core as preferred. */
> +	first_hk_core = topology_sibling_cpumask(cpu);
> +	cpu = cpumask_last(cpu_preferred_mask);
> +	if (cpu >= nr_cpu_ids)
> +		return;
> +
> +	/* Find the last CPU which doesn't belong to that first hk_core. */
> +	if (!cpumask_test_cpu(cpu, first_hk_core)) {
> +		target_cpu = cpu;
> +	} else {
> +		for_each_cpu_andnot(cpu, cpu_preferred_mask, first_hk_core)
> +			target_cpu = cpu;
> +	}
> +
> +	/* Only the first housekeeping core remains */
> +	if (target_cpu >= nr_cpu_ids)
> +		return;
> +
> +	for_each_cpu_and(cpu, topology_sibling_cpumask(target_cpu),
> +			 cpu_preferred_mask)
> +		set_cpu_preferred(cpu, false);
> +}
> +
> +/*
> + * Called when the steal governor detects no/low physical CPU contention.
> + * It finds the first active core outside of preferred mask and mark
> + * those CPUs as preferred.
> + *
> + * Must ensure preferred is subset of active.
> + */
> +static void increase_preferred_cpus(void)
> +{
> +	int first_cpu, cpu;
> +
> +	guard(cpus_read_lock)();
> +	first_cpu = cpumask_first_andnot(cpu_active_mask, cpu_preferred_mask);
> +
> +	/* All CPUs are preferred. Nothing to increase further */
> +	if (first_cpu >= nr_cpu_ids)
> +		return;
> +
> +	for_each_cpu_and(cpu, topology_sibling_cpumask(first_cpu),
> +			 cpu_active_mask)
> +		set_cpu_preferred(cpu, true);
> +}
> +
> +static bool preferred_cpus_valid(void)
> +{
> +	if (cpumask_empty(cpu_preferred_mask)) {
> +		pr_err("empty preferred mask. stopping\n");
> +		return false;
> +	}
> +
> +	if (!cpumask_subset(cpu_preferred_mask, cpu_active_mask)) {
> +		pr_err("preferred: %*pbl is not subset of active: %*pbl, stopping\n",
> +		       cpumask_pr_args(cpu_preferred_mask),
> +		       cpumask_pr_args(cpu_active_mask));
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void compute_preferred_cpus_work(struct work_struct *work)

Bad name. You're not only computing here, but actually adjusting the
preferred CPUs mask.

> +{
> +	u64 curr_steal, delta_steal, delta_ns, steal_ratio;
> +	ktime_t now;
> +
> +	now = ktime_get();
> +	delta_ns = ktime_to_ns(ktime_sub(now, sg_ctx.time));
> +
> +	if (unlikely(delta_ns < NSEC_PER_MSEC)) {
> +		pr_err_ratelimited("work scheduled too soon delta_ns: %llu\n", delta_ns);
> +		goto requeue_work;
> +	}
> +
> +	curr_steal = get_system_steal_time();
> +	delta_steal = curr_steal > sg_ctx.steal ? curr_steal - sg_ctx.steal : 0;
> +	sg_ctx.steal = curr_steal;
> +	sg_ctx.time = now;
> +
> +	/*
> +	 * steal_ratio = (delta_steal * 100*100)/(delta_ns * num_cpus())
> +	 * To avoid possible overflow, divide the denominator early.
> +	 * Note minimum interval is 100ms.
> +	 */
> +	delta_ns = max_t(u64, div_u64(delta_ns * get_system_cpus(), 10000), 1);
> +	steal_ratio = div64_u64(delta_steal, delta_ns);

So if:

Possible CPUs = 128
Active CPUs = 8
Steal on online CPUs = 50%
Steal on offline CPUS = 0%

Then calculated ratio would be:

        (50% × 8 + 0% * 120) / 128 = 3.125%

Instead of decreasing the number of preferred CPUs, you'll do nothing
under default thresholds, or even increase.

Have you tested your driver against such a configuration?

Also, I'm not quite sure how you'd handle a case when you have half
of CPUS in your core offlined, but you manage preferred mask per-core,
so you offline or online less CPUs than expected. Can you mention that
scenario in the documentation?

> +
> +	if (steal_ratio > sg_ctx.high_threshold)
> +		decrease_preferred_cpus();
> +	else if (steal_ratio <= sg_ctx.low_threshold)
> +		increase_preferred_cpus();
> +	else
> +		goto requeue_work;
> +
> +	if (!preferred_cpus_valid()) {
> +		restore_preferred_to_active();
> +		return;
> +	}
> +
> +requeue_work:
> +	schedule_delayed_work(&sg_ctx.work, sg_ctx.delay);
> +}
> +
>  static int __init steal_governor_init(void)
>  {
>  	if (sg_ctx.low_threshold >= sg_ctx.high_threshold) {
> @@ -117,6 +267,10 @@ static int __init steal_governor_init(void)
>  	}
>  
>  	sg_ctx.delay = msecs_to_jiffies(sg_ctx.interval_ms);
> +	INIT_DELAYED_WORK(&sg_ctx.work, compute_preferred_cpus_work);
> +	sg_ctx.steal = get_system_steal_time();
> +	sg_ctx.time = ktime_get();
> +	schedule_delayed_work(&sg_ctx.work, sg_ctx.delay);
>  	pr_info("enabled. interval: %ums, high_threshold: %u, low_threshold: %u\n",
>  		sg_ctx.interval_ms, sg_ctx.high_threshold, sg_ctx.low_threshold);
>  
> @@ -125,6 +279,7 @@ static int __init steal_governor_init(void)
>  
>  static void __exit steal_governor_exit(void)
>  {
> +	disable_delayed_work_sync(&sg_ctx.work);
>  	restore_preferred_to_active();
>  	pr_info("disabled\n");
>  }
> -- 
> 2.47.3