On Tuesday 19 May 2015 13:52:10 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.
Ok.
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. 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.
Yes, I'm sure this is a safe assumption.
When you change the code to ktime_us_delta, please also update the comment here.
(b) printk diffing the tv[i] and tv[i-1] values. The original printk statement seems to get the order wrong. This patch preserves that order.
I also think that's ok, but it would be nice to split this out as a separate patch that gets applied first, so just swap out the arguments in the timeval_usec_diff() definition as preparation and add an explanation why you think it was wrong.
@@ -2489,7 +2469,7 @@ static int dvb_frontend_ioctl_legacy(struct file *file, printk("%s(%d): switch delay (should be 32k followed by all 8k\n", __func__, fe->dvb->num); for (i = 1; i < 10; i++)
printk("%d: %d\n", i, timeval_usec_diff(tv[i-1] , tv[i]));
printk("%d: %d\n", i, (int) ktime_to_us(ktime_sub(tv[i-1], tv[i])));
This can still be simplified using ktime_us_delta().
stv0299_writeregI (state, 0x0c, reg0x0c | (last ? lv_mask : 0x50));@@ -443,7 +444,7 @@ static int stv0299_send_legacy_dish_cmd (struct dvb_frontend* fe, unsigned long printk ("%s(%d): switch delay (should be 32k followed by all 8k\n", __func__, fe->dvb->num); for (i = 1; i < 10; i++)
printk ("%d: %d\n", i, timeval_usec_diff(tv[i-1] , tv[i]));
printk("%d: %d\n", i, (int) ktime_to_us(ktime_sub(tv[i-1], tv[i])));
This, too.
The patch looks correct to me now, so just do the cosmetic changes I suggested above and submit a version 3.
Arnd