Re: [Y2038] [PATCH] dummy_hcd: replace timeval with timespec64

[Arnd Bergmann]

On Tuesday 15 September 2015 20:56:15 WEN Pingbo wrote:
> The millisecond of the last second will be normal if tv_sec is
> overflowed. But for y2038 consistency and demonstration purpose,
> and avoiding further risks, we still need to fix it here,
> to avoid similair problems.
> 
> Signed-off-by: Pingbo Wen <pingbo.wen@xxxxxxxxxx>
> Cc: Y2038 <y2038@xxxxxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Felipe Balbi <balbi@xxxxxx>

I just replied to the thread of the first mail, and it's good to see you had
the same thought about adding the usb folks to the Cc list.

When sending a new version of a patch you have already sent before, it's
common practice to use [PATCH v2] in the subject, so please do that for the
next version.

Your changelog text is better than the first version, but can still be improved.
I would at least mention that we want to remove all uses of 'timeval' from
device drivers in order to better scan for y2038 problems in the drivers that
still use them.

Also, you don't mention at all the discussion we had about real time vs.
monotonic time for this driver. I think using monotonic time (ktime_get_ts64())
would be more appropriate here, but whichever you choose, you should explain
in the changelog why you think that one is better than the other.
Most of the time, we end up changing from real time to monotonic time in the
same patch when converting a driver to avoid 32-bit time_t, so even you don't
change it, you should explain why not.

>  drivers/usb/gadget/udc/dummy_hcd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index 1379ad4..7be721dad 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -833,10 +833,10 @@ static const struct usb_ep_ops dummy_ep_ops = {
>  /* there are both host and device side versions of this call ... */
>  static int dummy_g_get_frame(struct usb_gadget *_gadget)
>  {
> -	struct timeval	tv;
> +	struct timespec64 tv;
>  
> -	do_gettimeofday(&tv);
> -	return tv.tv_usec / 1000;
> +	getnstimeofday64(&tv);
> +	return tv.tv_nsec / 1000000L;
>  }
>  

As in your other patch, I think the use of NSEC_PER_MSEC would make this
slightly more understandable.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/