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

[Shrikanth Hegde]

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.

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.

> Ya, it is a never-happen condition. I will use WARN_ON.