lkml.org 
[lkml]   [2015]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [Y2038] [PATCH v2] [media] dvb-frontend: Replace timeval with ktime_t
Date
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


\
 
 \ /
  Last update: 2015-05-20 16:41    [W:0.033 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site