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

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

Hi Yury.

On 7/14/26 6:17 PM, Yury Norov wrote:
> On Mon, Jul 13, 2026 at 10:43:15AM +0530, Shrikanth Hegde wrote:
>> Hi Yury,
>>
>> On 7/11/26 1:30 AM, Yury Norov wrote:
> 
> ...
> 
>>> 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().
>>>
>>
>> I thought about the extra work in function, but doesn't happen too often IMO.
> 
> 10ms is the shortest possible interval, 100 times in a second.

Even 10ms is too short i think. I am thinking to keep it 100ms. So
some meaningful number in terms of steal time accumulates.

10ms means only 1 tick on HZ=100 (which is a valid config even today).
100ms means at least 10 ticks.

That also reduces overhead concerns a bit as well.

> 
>> Also, it is specially not a misconfiguration for increase.
>> So i have kept it stateless for the below reason.
>>
>> - Under typical operation of this driver, user will enable it once.
>> - Once enabled, user will use their VM as usual.
>> - Majority of the time the steal time will be less.
>> - workload are bursty in nature.
> 
> Not necessarily.
> 
>> - Occasionally many VM will have high utilization and there will be steal time.
>>    This lasts for sometime.
>> - After workload completes, steal time goes low again.
>> - Cycle could repeat after extended low steal time duration.
>>
>> So when the steal time is low, though driver is enabled, doesn't mean it
>> is mis-configured. Just that there is contention and driver has nothing to
>> do. So, adding print there could easily consume the console.
>>
>> Similarly, there could situations, where decrease cannot happen though there is
>> high steal time, Though they are corner cases.
> 
> OK, that makes sense. If one needs to fine-tune the driver's
> thresholds, he'd monitor the preferred cpumasks statistics.
> 
>> For example,
>> - one small/few VMs have not enabled the driver. steal time could be high, but this
>>    VM has already down to one core. It can't decrease any further.
>> - Though all VMs have enabled the feature, but task running is not FAIR class. Though
>>    steal time shows high.
>>
>> Hitting only one core or all cores isn't necessarily a misconfiguration.
>> It is a possible behavior during severe contention or complete idle system.
>>
>> we need to continuously monitor steal time so that it can expand/contract the
>> based on current situation. If we stop calling the functions, natural expand/contract
>> will not happen. There is no interrupt which arrives due to high/low steal time where
>> we can kick start the driver again. Also it is a difficult ask for user to keep enabling
>> or disabling the driver.
>>
>> Since this can be called at minimal once in 10ms, I guess we can incur the additional
>> overheads to keep the logic simple and stateless. What do you think?
> 
> OK, it's your design.
> 

Ok.

I will keep it stateless for now.
If the need arises, we can bring in more towards that.

>> PS: I will remove that additional SM_DIR as you suggested in other reply. That keeps
>> it all stateless.