Re: [PATCH v6 20/23] virt/steal_monitor: Act on steal values at regular intervals
From: Shrikanth Hegde
Date: Thu Jul 09 2026 - 00:39:11 EST
On 7/9/26 1:29 AM, Yury Norov wrote:
On Wed, Jul 08, 2026 at 10:25:09PM +0530, Shrikanth Hegde wrote:
On 7/8/26 9:43 PM, Yury Norov wrote:
On Wed, Jul 08, 2026 at 04:03:18PM +0530, Shrikanth Hegde wrote:
On 7/7/26 12:46 PM, Shrikanth Hegde wrote:
On 7/7/26 2:03 AM, Yury Norov wrote:
On Wed, Jul 01, 2026 at 07:46:51PM +0530, Shrikanth Hegde wrote:
This is the steal_monitor core functionality done in periodic work
- Calculate the steal_ratio. It is multiplied by 100 to consider the
fractional values of steal time. I.e 10 means 0.1% steal time.
- 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.
Signed-off-by: Shrikanth Hegde <sshegde@xxxxxxxxxxxxx>
---
v5->v6:
- Address u64 overflow concerns.
drivers/virt/steal_monitor/sm_core.c | 33 ++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/virt/steal_monitor/sm_core.c
b/drivers/virt/ steal_monitor/sm_core.c
index b499faa61010..7b7435f79b85 100644
--- a/drivers/virt/steal_monitor/sm_core.c
+++ b/drivers/virt/steal_monitor/sm_core.c
@@ -34,6 +34,37 @@ MODULE_PARM_DESC(low_threshold,
static void compute_preferred_cpus_work(struct work_struct *work)
{
+ u64 curr_steal, delta_steal, delta_ns, steal_ratio;
+ ktime_t now;
+
+ curr_steal = get_system_steal_time();
+ now = ktime_get();
+
+ /* get the deltas */
+ delta_steal = curr_steal > sm_core_ctx.prev_steal ?
+ curr_steal - sm_core_ctx.prev_steal : 0;
+ delta_ns = max_t(u64, ktime_to_ns(ktime_sub(now,
sm_core_ctx.prev_time)), 1);
The below return on '!delta_ns' makes this max(...) useless, right?
Regardless, if the time between 2 measures is less then 1ns, I
believe, the whole measure is not trustworthy
Seeing sashiko comment on divide by zero, i forgot I had max above there :(
+
+ /* Update for next calculation */
+ sm_core_ctx.prev_steal = curr_steal;
+ sm_core_ctx.prev_time = now;
So below return should go prior to this update, because 'now' is actually
the same as 'prev_time', right?
I don't understand why 'now' can be so close to prev_time, because
you've scheduled this callback on the regular interval. But if that's
possible, can you explain that and do like this at the very beginning
of the function:
I don't think it is possible.
now = ktime_get();
if (unlikely(now < sm_core_ctx.prev_time +
sm_core_ctx.interval / 2)) {
pr_warn(...);
return;
}
Pretty much, just have to requeue the work.
And if it's a never-happen condition, just use WARN_ON().
I have decided keep just max_t() to ensure no divide by zero happens.
Issue with putting sm_core_ctx.interval_ms check is that, one may edit
it while the work is queued, but not yet called. It would print wrongly
in that case.
Who is that 'one'? If it's userspace, you can simply disallow changing
the interval. If it's kernel, you're already compromised, and
everything is not trustworthy.
I would simply disallow to change any parameters while the driver is
loaded. If I need to change something - I unload it and load with new
interval, or whatever.
It is user writing to /sys/module/steal_monitor/parameters/interval_ms
that i thought of. So user.
If we disable then those checks can be valid.
Even if work function gets called prematurely, it should be fine since theIf there's 1 nanosecond between two calls, your statistics is very
steal also increases accordingly. No issues in delta logic.
So capping to 1 to ensure no divide by 0 happens due to scaling of
delta_ns is good enough.
inaccurate, pretty much a noise. You want to make decision based on
noise. That's simply wrong.
yes, that makes sense too. I have kept interval_ms/8 as safe bet.
Return will not queue the timer and effectively disables it.
ktime_t now;
now = ktime_get();
delta_ns = ktime_to_ns(ktime_sub(now, sm_core_ctx.prev_time));
if (unlikely(delta_ns < sm_core_ctx.interval_ms * ((u64)NSEC_PER_MSEC >> 3))) {
pr_err("work scheduled too soon delta_time: %llu ns. Abort steal_monitor.\n",
delta_ns);
guard(cpus_read_lock)();
cpumask_copy(&__cpu_preferred_mask, cpu_active_mask);
return;
}
Don't overthink it. Simply don't allow user to change the parameters
while the driver is running. It may only cause troubles to those
compiling the driver into the Image. Those compiling it as a module
will only have to reload the driver. This is how most of the kernel
works.
Ok. I have made it that way.
I will just put a pr_err for delta_ns less than 1ms. If anyone hits it,
then we can bring in the logic to abort etc.
PS: I wasn;t able to hit it even with 1ms as interval and check for delta_ns as (1ms/8)
So, when the driver is loaded, make sanity checks for all user parameters.
Interval, for example must be between 10ms and 10s. If user wants 20s,
I have it as 10ms and 100s in current v7.
pr_err() something and fail to load. After the driver is loaded, don't
allow to change the params exactly because it may bring certain type of
errors in the logic, and you don't want to mitigate them.
If there will be a real need to adjust parameters on the fly - it will
be another interesting topic (mostly interesting why user wants this).
Makes sense. If the need arise, we will figure something out.
I have kept it simple for now.
now = ktime_get();
delta_ns = ktime_to_ns(ktime_sub(now, sm_core_ctx.prev_time));
if (unlikely(delta_ns < NSEC_PER_MSEC)) {
pr_err_ratelimited("steal_monitor: work scheduled too soon delta_ns: %llu\n",
delta_ns);
goto requeue_work;
}