Re: [PATCH v7 10/12] virt/steal_monitor: Provide functions for managing steal values

[Yury Norov <ynorov@nvidia.com>]

On Fri, Jul 10, 2026 at 03:26:46AM +0530, Shrikanth Hegde wrote:
> Provide functions which is going to be used in the periodic work
> function to calculate and handle steal time values.
> 
> get_system_steal_time()
> - steal monitor takes global view of steal time instead of individual
>   vCPU. For this collect overall steal values across all the vCPUs or
>   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.
> 
> 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 must stay 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.
> 
> 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.
> 
> get_num_cpus_steal_ratio()
> - This method informs the steal_monitor core, how many CPUs it needs to
>   consider 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>
> ---
> v6->v7:
> - Combined patches which added helper functions.
> - Use possible CPUs for steal value calculations. 
> 
>  drivers/virt/steal_monitor/Makefile   |   2 +-
>  drivers/virt/steal_monitor/defaults.c | 100 ++++++++++++++++++++++++++
>  drivers/virt/steal_monitor/sm_core.h  |   8 +++

What for do you split functionality into sm_core and default? There's
no non-default implementation, right?

I'd just put everything in drivers/virt/steal_monitor.c. It would be
~300 LOCs file - quite bearable.

>  3 files changed, 109 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/virt/steal_monitor/defaults.c
> 
> diff --git a/drivers/virt/steal_monitor/Makefile b/drivers/virt/steal_monitor/Makefile
> index bd7d120a79b5..273a6dd59fea 100644
> --- a/drivers/virt/steal_monitor/Makefile
> +++ b/drivers/virt/steal_monitor/Makefile
> @@ -3,4 +3,4 @@
>  # Steal time monitor to alter preferred CPU state.
>  obj-$(CONFIG_STEAL_MONITOR) += steal_monitor.o
>  
> -steal_monitor-y := sm_core.o
> +steal_monitor-y := sm_core.o defaults.o
> diff --git a/drivers/virt/steal_monitor/defaults.c b/drivers/virt/steal_monitor/defaults.c
> new file mode 100644
> index 000000000000..d4b016317554
> --- /dev/null
> +++ b/drivers/virt/steal_monitor/defaults.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Base file contains the default implementations.
> + *
> + * Copyright (C) 2026 IBM
> + * Author: Shrikanth Hegde <sshegde@linux.ibm.com>
> + */
> +#include "sm_core.h"
> +
> +/*
> + * Returns steal time of the full system.
> + * Compute collective steal time across all possible CPUs.
> + */
> +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;
> +}
> +
> +/*
> + * Returns number of CPUs to consider for steal ratio.
> + * Return possible CPUs.
> + */
> +unsigned int get_num_cpus_steal_ratio(void)
> +{
> +	return num_possible_cpus();
> +}
> +
> +/*
> + * Take action to decrease preferred CPUs.
> + *
> + * Decrease the preferred CPUs by 1 core.
> + * Take out the last core in the active & preferred.
> + *
> + * Must ensure
> + * - least one housekeeping core is always kept as preferred
> + * - preferred is always subset of active.
> + */
> +void decrease_preferred_cpus(struct steal_monitor *ctx)
> +{
> +	int tmp_cpu, first_hk_cpu, last_cpu;
> +	const struct cpumask *first_hk_core;
> +	int target_cpu = nr_cpu_ids;
> +
> +	guard(cpus_read_lock)();
> +	first_hk_cpu = cpumask_first_and(housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
> +					 cpu_preferred_mask);

Nit: you can return here if nothing found, and save on the 2nd
traverse.

> +	last_cpu = cpumask_last(cpu_preferred_mask);
> +
> +	if (first_hk_cpu >= nr_cpu_ids || last_cpu >= nr_cpu_ids)
> +		return;
> +
> +	/* Always leave first housekeeping core as preferred. */
> +	first_hk_core = topology_sibling_cpumask(first_hk_cpu);
> +
> +	/* Find the last CPU which doesn't belong to that first hk_core. */
> +	if (!cpumask_test_cpu(last_cpu, first_hk_core)) {
> +		target_cpu = last_cpu;
> +	} else {
> +		for_each_cpu_andnot(tmp_cpu, cpu_preferred_mask, first_hk_core)
> +			target_cpu = tmp_cpu;
> +	}
> +
> +	/* Only the first housekeeping core remains */
> +	if (target_cpu >= nr_cpu_ids)
> +		return;
> +
> +	for_each_cpu_and(tmp_cpu, topology_sibling_cpumask(target_cpu),
> +			 cpu_preferred_mask)
> +		set_cpu_preferred(tmp_cpu, false);

I think it should return status: if the function can't disable CPUs
now, it would be a good hint for the caller that it would be useless
to call it again.

You may keep status in struct steal_monitor like:

        if (steal_ratio > sm_core_ctx.high_threshold)  {
                if (sm_core_ctx->status | CANT_DECREASE) {
                        pr_something();
                else
                        sm_core_ctx->status = decrease();

It would be a good hint to user that he has the driver misconfigured,
and save the driver extra work. Same for increase().

> +}
> +
> +/*
> + * Take action to increase preferred CPUs.
> + *
> + * Increase the preferred CPUs by 1 core.
> + * Add the first core in active & !preferred
> + *
> + * Must ensure preferred is subset of active.
> + */
> +void increase_preferred_cpus(struct steal_monitor *ctx)
> +{
> +	int first_cpu, tmp_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(tmp_cpu, topology_sibling_cpumask(first_cpu),
> +			 cpu_active_mask)
> +		set_cpu_preferred(tmp_cpu, true);
> +}
> diff --git a/drivers/virt/steal_monitor/sm_core.h b/drivers/virt/steal_monitor/sm_core.h
> index 8bbb606add99..ee68cd8b1944 100644
> --- a/drivers/virt/steal_monitor/sm_core.h
> +++ b/drivers/virt/steal_monitor/sm_core.h
> @@ -11,6 +11,9 @@
>  #include <linux/cpumask.h>
>  #include <linux/workqueue.h>
>  #include <linux/ktime.h>
> +#include <linux/kernel_stat.h>
> +#include <linux/topology.h>
> +#include <linux/sched/isolation.h>
>  
>  struct steal_monitor {
>  	struct delayed_work	work;
> @@ -24,4 +27,9 @@ struct steal_monitor {
>  
>  extern struct steal_monitor sm_core_ctx;
>  
> +u64 get_system_steal_time(void);
> +unsigned int get_num_cpus_steal_ratio(void);
> +void increase_preferred_cpus(struct steal_monitor *ctx);
> +void decrease_preferred_cpus(struct steal_monitor *ctx);
> +
>  #endif /* __VIRT_STEAL_CORE_H */
> -- 
> 2.47.3