Re: [PATCH v6 18/23] virt/steal_monitor: Provide default method to inc/dec preferred CPUs

[Yury Norov]

On Wed, Jul 01, 2026 at 07:46:49PM +0530, Shrikanth Hegde wrote:
> These methods will be used by the steal_monitor core in subsequent
> patches. Default implementation are likely good enough for most archs.
> 
> 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.
> increase_preferred_cpus() - Called when there is low steal time. It needs
> to decide which CPUs to mark as preferred and set that state.
> 
> Default Implementations:
> decrease_preferred_cpus()
> - Get first housekeeping CPU and its core mask. Mark it as
>   protected core. This helps to keep at least one core as preferred.

Is there any measurable difference to manage preferred CPUs per-core
vs per-cpu? If not, I'd choose to per-cpu version as it's simpler.

>   This is to be safe under non-normal cases.
> - Find the last CPU outside of this protected core mask. (target CPU)
>   This works for cases where one may specify nohz_full= for last set of
>   CPUs as well.

And doesn't work for cases where one specifies nohz_full for the first
set of CPUs. If you want to do it properly, you need two passes:
1. Search for preferred nohz_full candidates, and 
2. If no such CPU found, search for any preferred CPU.      

> - If no such CPU exits, then only housekeeping core remains. Bail out.
> - Based on that target CPU, get its sibling and mark them as
>   non-preferred. If they are nohz_full, enable the tick.
>   push mechanism relies on sched_tick.
> 
> increase_preferred_cpus()
> - Get the first active non-preferred CPUs. This likely is the last
>   set of CPUs being marked as non-preferred.
> - If there is no such CPU, i.e preferred is same as active. Nothing
>   todo further.
> - If not, get the siblings of that core and mark them as preferred.
>   Note that clearing the tick isn't needed as that would be handled via
>   sched_can_stop_tick.
> 
> Using core instead of individual CPUs give better numbers as SMT is
> quite common and some hypervisor such as powerVM does core scheduling.
> 
> Note: This doesn't do any NUMA splicing to keep the code simpler and
> minimal overhead. current code expects CPUs spread unformly
> across NUMA nodes.
> 
> Signed-off-by: Shrikanth Hegde <sshegde@xxxxxxxxxxxxx>
> ---
> v5->v6:
> - Make it work for all cases when nohz_full= may be specified.
> 
>  drivers/virt/steal_monitor/defaults.c | 74 +++++++++++++++++++++++++++
>  drivers/virt/steal_monitor/sm_core.h  |  2 +
>  2 files changed, 76 insertions(+)
> 
> diff --git a/drivers/virt/steal_monitor/defaults.c b/drivers/virt/steal_monitor/defaults.c
> index 6681f9938f6a..4e2e5b233948 100644
> --- a/drivers/virt/steal_monitor/defaults.c
> +++ b/drivers/virt/steal_monitor/defaults.c
> @@ -26,3 +26,77 @@ u64 __weak get_system_steal_time(void)
>  
>  	return total_steal;
>  }
> +
> +/*
> + * Default implementation of decrementing the preferred CPUs based on steal
> + * time. This is simple logic and decrease the preferred CPUs by 1 core.
> + * It takes out the last core in the active & preferred.
> + *
> + * Ensure at least one housekeeping core is always kept as preferred
> + *
> + * Could be overwritten by arch specific handling. Arch must ensure
> + * preferred is always subset of active.
> + */
> +
> +#define get_core_mask(cpu) topology_sibling_cpumask(cpu)

useless redefinition.

> +
> +void __weak decrease_preferred_cpus(struct steal_monitor *ctx)
> +{
> +	int tmp_cpu, first_hk_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_active_mask);

Why active mask? If you want to preserve a cpu in preferred mask,
search the cpu_preferred_mask.

Can you explain why HK_TYPE_KERNEL_NOISE? Honestly, I don't
understand the idea behind this logic.

> +
> +	if (first_hk_cpu >= nr_cpu_ids)
> +		return;
> +
> +	first_hk_core = get_core_mask(first_hk_cpu);
> +
> +	/* Always leave first housekeeping core as preferred. */
> +	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;
> +
> +	/*
> +	 * set tick bit for nohz_full CPU to push the task out. Once the tasks
> +	 * are pushed out, bit will be cleared if there are no tasks.
> +	 */
> +
> +	for_each_cpu_and(tmp_cpu, get_core_mask(target_cpu), cpu_active_mask) {

Why active mask here? It should be preferred, I guess? If you traverse
the active mask, you're at least doing an extra work by disabling the
already disabled CPUs, and probably have some side effects by calling
tick_nohz_dep_set_cpu() twice.

> +		set_cpu_preferred(tmp_cpu, false);
> +		if (tick_nohz_full_cpu(tmp_cpu))
> +			tick_nohz_dep_set_cpu(tmp_cpu, TICK_DEP_BIT_SCHED);
> +	}
> +}
> +
> +/*
> + * Default implementation of incrementing preferred CPUs based on steal
> + * time. This is simple logic and increases the preferred CPUs by 1 core.
> + * It adds the first core in active & !preferred
> + *
> + * Nothing to do if active == preferred
> + *
> + * Could be overwritten by arch specific handling. Arch must ensure
> + * preferred is subset of active.
> + */
> +void __weak 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, get_core_mask(first_cpu), cpu_active_mask)

Here I agree, it should be 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 634c9f5a2610..030f6236c38e 100644
> --- a/drivers/virt/steal_monitor/sm_core.h
> +++ b/drivers/virt/steal_monitor/sm_core.h
> @@ -26,5 +26,7 @@ struct steal_monitor {
>  extern struct steal_monitor sm_core_ctx;
>  
>  u64 get_system_steal_time(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