Re: [PATCH v6 20/23] virt/steal_monitor: Act on steal values at regular intervals

[Yury Norov]

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.

> Even if work function gets called prematurely, it should be fine since the
> 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.
 
If there's 1 nanosecond between two calls, your statistics is very
inaccurate, pretty much a noise. You want to make decision based on
noise. That's simply wrong.
 
> > Ya, it is a never-happen condition. I will use WARN_ON.
> >