On Monday 11 May 2015 08:07:51 Tina Ruchandani wrote:
> struct timeval uses a 32-bit seconds representation which will
> overflow in the year 2038 and beyond. This patch replaces
> the usage of struct timeval with ktime_t which is a 64-bit
> timestamp and is year 2038 safe.
> This patch is part of a larger attempt to remove all instances
> of 32-bit timekeeping variables (timeval, timespec, time_t)
> which are not year 2038 safe, from the kernel.
> The patch is a work-in-progress - correctness of the following
> changes is unclear:
> (a) Usage of timeval_usec_diff - The function seems to subtract
> usec values without caring about the difference of the seconds field.
> There may be an implicit assumption in the original code that the
> time delta is always of the order of microseconds.
Right, in particular, it does handle the seconds field overflowing
once, but it breaks when the difference is larger than one second.
> The patch replaces the usage of timeval_usec_diff with
> ktime_to_us(ktime_sub()) which computes the real timestamp difference,
> not just the difference in the usec field.
As in the other patches, ktime_us_delta() would look nicer, but it's
equally correct.
> * Sleep until gettimeofday() > waketime + add_usec
> * This needs to be as precise as possible, but as the delay is
> * usually between 2ms and 32ms, it is done using a scheduled msleep
> * followed by usleep (normally a busy-wait loop) for the remainder
> */
> -void dvb_frontend_sleep_until(struct timeval *waketime, u32 add_usec)
> +void dvb_frontend_sleep_until(ktime_t waketime, u32 add_usec)
> {
> - struct timeval lasttime;
> + ktime_t lasttime;
> s32 delta, newdelta;
>
> - timeval_usec_add(waketime, add_usec);
> + ktime_add_us(waketime, add_usec);
>
> - do_gettimeofday(&lasttime);
> - delta = timeval_usec_diff(lasttime, *waketime);
> + lasttime = ktime_get_real();
> + delta = ktime_to_us(ktime_sub(lasttime, waketime));
> if (delta > 2500) {
> msleep((delta - 1500) / 1000);
> - do_gettimeofday(&lasttime);
> - newdelta = timeval_usec_diff(lasttime, *waketime);
> + lasttime = ktime_get_real();
> + newdelta = ktime_to_us(ktime_sub(lasttime, waketime));
> delta = (newdelta > delta) ? 0 : newdelta;
> }
There is a bug you introduced: The original function added add_usec
to the value pointed to by waketime. Changing this from a pointer
to a scalar argument means that the value is only updated in the
dvb_frontend_sleep_until() function, but not inside of the caller.
In order to preserve the behavior, you have to leave waketime as a pointer.
Arnd
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
