Re: [Y2038] [PATCH] aoe: Use 64-bit timestamp in frame

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 12 May 2015 21:23:04 Ed Cashin wrote:
> On 05/12/2015 07:14 AM, Arnd Bergmann wrote:
> > On Tuesday 12 May 2015 11:44:21 Arnd Bergmann wrote:
> >> There are of course multiple ways to do this. One way would be to
> >> change the code to work on 32-bit nanoseconds instead of 32-bit
> >> microseconds. This requires proving that the we cannot exceed
> >> 4.29 seconds of round-trip time in calc_rttavg().
> >> Is that a valid assumption or not?
> >>
> >> If not, we could replace do_gettimeofday() with ktime_get_ts64().
> >> This will ensure we don't need a 64-bit division when converting
> >> the ts64 to a 32-bit microsecond value, and combined with the
> >> conversion is still no slower than do_gettimeofday(), and it
> >> still avoids the double bookkeeping because it uses a monotonic
> >> timebase that is robust against settimeofday.
> > Two other approaches that occurred to me later:
> >
> > - introduce common ktime_get_ms(), ktime_get_us(), ktime_get_real_ms()
> >    and ktime_get_real_is() interfaces, to match the other interfaces
> >    we already provide. These could be done as efficiently or better
> >    than what aoe does manually today.
> >
> > - change the timebase that is used for the computations in aoe to use
> >    scaled nanoseconds instead of microseconds. Using
> >
> >    u32 time = ktime_get_ns() >> 10;
> >
> >    would give you a similar range and precision as microseconds, but
> >    completely avoid integer division. You could also use a different
> >    shift value to either extend the range beyond 71 minutes, or the
> >    extend the precision to something below a microsecond. This would
> >    be the most efficient implementation, but also require significant
> >    changes to the driver.
> >
> 
> That is an interesting idea.  People do care about aoe_deadsecs being
> pretty accurate, so there would need to be a way to make that remain
> accurate.  The driver will fail outstanding I/O to the target and mark it
> as "down" after unsuccessfully retransmitting commands to the target
> for a number of seconds equal to aoe_deadsecs.
> 
> As to the efficient ktime_get_us idea, that sounds appealing since you
> mention that they would be efficient.
> 
> Thanks for the analysis.

Shall we do the ktime_get_us() approach then? It still requires a 
32-bit division like do_gettimeofday(), so it will not be as efficient
as the shifted nanoseconds.

As for the aoe_deadsecs computation, converting the aoe_deadsec
module parameter into scaled nanoseconds can be done at module
load time, and that way you also save the integer division you
currently do for each frame in rexmit_timer() to turn the
microseconds into seconds.

	Arnd