Messages in this thread | | | Date | Wed, 30 Mar 2022 15:40:55 +0200 (CEST) | From | Anna-Maria Behnsen <> | Subject | Re: [PATCH v3 1/2] timer: add a function to adjust timeouts to be upper bound |
| |
On Wed, 30 Mar 2022, Artem Savkov wrote:
> Current timer wheel implementation is optimized for performance and > energy usage but lacks in precision. This, normally, is not a problem as > most timers that use timer wheel are used for timeouts and thus rarely > expire, instead they often get canceled or modified before expiration. > Even when they don't, expiring a bit late is not an issue for timeout > timers. > > TCP keepalive timer is a special case, it's aim is to prevent timeouts, > so triggering earlier rather than later is desired behavior. In a > reported case the user had a 3600s keepalive timer for preventing firewall > disconnects (on a 3650s interval). They observed keepalive timers coming > in up to four minutes late, causing unexpected disconnects. > > This commit adds upper_bound_timeout() function that takes a relative > timeout and adjusts it based on timer wheel granularity so that supplied > value effectively becomes an upper bound for the timer. >
I think there is a problem with this approach. Please correct me, if I'm wrong. The timer wheel index and level calculation depends on timer_base::clk. The timeout/delta which is used for this calculation is relative to timer_base::clk (delta = expires - base::clk). timer_base::clk is not updated in sync with jiffies. It is forwarded before a new timer is queued. It is possible, that timer_base::clk is behind jiffies after forwarding because of a not yet expired timer.
When calculating the level/index with a relative timeout, there is no guarantee that the result is the same when actual enqueueing the timer with expiry = jiffies + timeout .
Thanks,
Anna-Maria
| |