Messages in this thread | | | Date | Thu, 30 Sep 2021 15:26:42 +0200 | From | Peter Zijlstra <> | Subject | Re: [RFC] hrtimer: Fix the hrtimer_forward to use correct delta for expiry time |
| |
On Thu, Sep 30, 2021 at 06:22:19PM +0530, Athira Rajeev wrote: > Hrtimer uses "hrtimer_forward" function to forward the timer > expiry. This function takes the "time" from when to forward > and how much "interval" to forward as inputs. In some cases, it > is observed that though correct interval is given to forward the > timer, the expiry time is set to value less than expected interval. > This will cause the timer to expire ahead of expected expiry time. > Before updating the timer expiry value, hrtimer_forward checks to > see if there is any delta between the time from when to forwad > and previously set expiry. And this behaviiour is observed when > delta is large value. > > For example, consider case where we want to forward the expiry > from "now" to 10 milliseconds. Below is trace prints captured by > dumping the values used in "hrtimer_forward". And this instance is > captured while forwarding timer from perf event multiplexing code > which uses hrtimer. > > <<>> > [001] d.... 304.118944: perf_mux_hrtimer_restart: Restarting timer from perf_mux_hrtimer_restart > [001] d.... 304.118945: hrtimer_forward: In nanoseconds, now is 303938589749, delta is 52868589749, hrtimer_get_expires(timer) is 251070000000, interval is 10000000 > [001] d.... 304.118945: hrtimer_forward: Caller is perf_mux_hrtimer_restart+0xb0/0x120 > [001] d.... 304.118946: hrtimer_forward: Caller's caller is merge_sched_in+0x268/0x4e0 > [001] d.... 304.118946: hrtimer_forward: orun is 5286 > [001] d.... 304.118947: hrtimer_forward: In hrtimer_add_expires_ns, before ktime_add_ns timer->node.expires in ns is 251070000000, timer->_softexpires is 251070000000, ns is 52860000000 > [001] d.... 304.118948: hrtimer_forward: In hrtimer_add_expires_ns, after ktime_add_ns timer->node.expires in ns is 303930000000, timer->_softexpires is 303930000000 > [001] d.... 304.118948: hrtimer_forward: In hrtimer_add_expires, in nanoseconds, before ktime_add_safe, timer->node.expires in ns is 303930000000, timer->_softexpires is 303930000000 > [001] d.... 304.118949: hrtimer_forward: In hrtimer_add_expires, in nanoseconds, after ktime_add_safe, timer->node.expires in ns is 303940000000, timer->_softexpires is 303940000000 > [001] d.... 304.118949: hrtimer_forward: After hrtimer_add_expires, hrtimer_get_remaining in nanoseconds is 1405169 > > <<>> > > In this example, > timer->node.expires = 251070000000 ns ( previously set expiry ) > now = 303938589749 ns > delta = 52868589749 ns > > Here delta is "52868589749" which is greater than the interval to > forward ( ie 10000000 ns ). Hence hrtimer_forwards adds this difference > to present timer->node.expires in hrtimer_add_expires_ns() function. But > instead of using actual "delta", code is using (delta/interval * interval) > as below: > > <<>> > s64 incr = ktime_to_ns(interval); > orun = ktime_divns(delta, incr); > hrtimer_add_expires_ns(timer, incr * orun); > <<>> > > Hence we are actually using "orun * 10000000". In this example, it will be > "52860000000" since "orun" does not include the mod value. Here we are > missing "8589749" nanoseconds in the timer expiry. Hence, the final expiry > time will be set to "303940000000". > > Since we are not using exact delta, the hrtime remaining will be > around 1405169 nanoseconds and will cause the timer to expire in 1.4 ms > instead of 10 ms. Fix this by using "delta" instead of using the divided > value. >
You misunderstand, the behaviour is correct and expected. What hrtimer_forward_now() does is guarantee the expiry time ends up on an integer multiple of @interval.
This is important to achieve periodic timers. Your patch would cause period drift.
| |