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

[Shrikanth Hegde <sshegde@linux.ibm.com>]

Hi Yury,

On 7/4/26 2:41 AM, Yury Norov wrote:
> 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.
> 

Yes, per-core is much better.
Since powerpc is SMT8 and phyp does core scheduling, taking on/off a full core makes a difference.

Even for other arch having it per core (SMT2 is being most common) it better IMO.
It allows to achieve the same level of packing without aggressive interval_ms.

>>    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.
> 

It works, in that case protected core is one outside of nohz_full list.

I have tried on the system.
I have 480 CPUs. (60 cores SMT8) I see this happening.

If nohz_full=<0-239> protected core will be 240-247
(CPUs 240-479 are part of HK_TYPE_KERNEL_NOISE)
if nohz_full=<240-479> protected core is 0-7
(CPUs 0-239 are part of HK_TYPE_KERNEL_NOISE)

If i pass nohz_full=0-479, then i see below in dmesg.
Housekeeping: must include one present CPU, using boot CPU:0
cat nohz_full
1-479
In that case too, protected core is 0-7

>> - 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@linux.ibm.com>
>> ---
>> 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.

that was keep for loop under 80 chars. I can drop it.

> 
>> +
>> +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.

HK_TYPE_KERNEL_NOISE logic is, kernel ensures atleast one CPU is always left and it
can't be hotplugged. So use that core for the protected core, as leaving atleast one
core as preferred is better.

Whole logic (this patch and Patch 23 is to ensure that) is leave that core as preferred.

For example, I have 480 CPUs. (60 cores SMT8)

If nohz_full=<0-239> protected core will be 240-247
(CPUs 240-479 are part of HK_TYPE_KERNEL_NOISE)

if nohz_full=<240-479> protected core is 0-7
(CPUs 0-239 are part of HK_TYPE_KERNEL_NOISE)


I see that happening with the patch series.

> 
>> +
>> +	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.

Ok. I think preferred is better here.

But active also shouldn't cause any issue i think, since target_cpu is expected
to be a preferred CPU. So there is no double disable.

If there is was hotplug done, then it won;t be there in active too.
It takes cpus_read_lock to be safe against hotplug happening in parallel.

> 
>> +		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.

Yes.

> 
>> +		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