Re: [RFC PATCH v3 21/21] sched/cputime: Cope with steal time going backwards or negative

From: Paul Durrant
Date: Fri May 24 2024 - 10:25:45 EST


On 22/05/2024 01:17, David Woodhouse wrote:
From: David Woodhouse <dwmw@xxxxxxxxxxxx>

In steal_account_process_time(), a delta is calculated between the value
returned by paravirt_steal_clock(), and this_rq()->prev_steal_time which
is assumed to be the *previous* value returned by paravirt_steal_clock().

However, instead of just assigning the newly-read value directly into
->prev_steal_time for use in the next iteration, ->prev_steal_time is
*incremented* by the calculated delta.

This used to be roughly the same, modulo conversion to jiffies and back,
until commit 807e5b80687c0 ("sched/cputime: Add steal time support to
full dynticks CPU time accounting") started clamping that delta to a
maximum of the actual time elapsed. So now, if the value returned by
paravirt_steal_clock() jumps by a large amount, instead of a *single*
period of reporting 100% steal time, the system will report 100% steal
time for as long as it takes to "catch up" with the reported value.
Which is up to 584 years.

But there is a benefit to advancing ->prev_steal_time only by the time
which was *accounted* as having been stolen. It means that any extra
time truncated by the clamping will be accounted in the next sample
period rather than lost. Given the stochastic nature of the sampling,
that is more accurate overall.

So, continue to advance ->prev_steal_time by the accounted value as
long as the delta isn't egregiously large (for which, use maxtime * 2).
If the delta is more than that, just set ->prev_steal_time directly to
the value returned by paravirt_steal_clock().

Fixes: 807e5b80687c0 ("sched/cputime: Add steal time support to full dynticks CPU time accounting")
Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
---
kernel/sched/cputime.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)


Reviewed-by: Paul Durrant <paul@xxxxxxx>