Re: [PATCH v3 1/2] timer: add a function to adjust timeouts to be upper bound

[Anna-Maria Behnsen]

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