On Sunday 13 September 2015 18:35:40 WEN Pingbo wrote:
> - what is the time_t/timeval/timespec type used for in this driver
> the timeval in dummy_g_get_frame is used for getting frame number in every
> ms.
>
> - is this broken in 2038 or not
> No. The millisecond of the last second will be normal if tv_sec is
> overflowed. But for consistency purpose, and avoiding further risks, I
> have replaced timeval with timespec64.
Hi Pingbo,
The patch and your findings so far look good. As a changelog comment
however, the text needs improvement and should not be written in
question/answer style. Instead, try to come up with complete sentences
that explain the current state of the driver, why we want to change
it and how your patch addresses the problem.
It's quite likely that I will keep commenting mostly on your
changelog texts, but they are immensely important to get right
if you want to get patches merged. Basically you need to "sell" your
patch to the maintainer and explain why they really want to apply
it, when it's easier for them to not touch the driver to avoid
introducing a regression when they find an excuse not to apply the
patch. ;-)
> - does the driver use monotonic or real time
> Real time. But only microsecond part is used.
>
> - if it uses real time, should it use monotonic time instead
> Monotonic time will be ok if it can meet the precise requirement(us).
Your patch picks the easy approach by leaving the driver to use real
time. This clearly has a lower risk of regressions, which is good, but
you should come up with an argument on which of the two is the better
choice here.
As a background:
- The precision of real time and monotonic time is identical
- Both the microsecond/nanosecond and the second portion are different,
as monotonic time starts at the moment that the machine is booted,
and there is a constant offset between the two.
- Whenever a time value has to be synchronized between different machines,
we have to use real time (this does not seem to be the case here).
- "real" time can jump forward or backward in some cases. If any of these
would cause problems in the driver, you should use monotonic time instead:
- root calls "settimeofday"
- ntp sets the time because of an overly large offset to the
official time
- a "leap second" occurs
- both real and monotonic time can be accelerated or slowed down by ntp,
in order to stay in sync with the network time. If you instead need the
time to tick in sync with the CPU or bus clock generator, there is
"monotonic_raw" time. We rarely need this.
- If there are multiple code locations that access the same time fields,
they obviously need to use the same time base.
Arnd
> Signed-off-by: WEN Pingbo <[email protected]>
> Cc: Y2038 <[email protected]>
> ---
> 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;
> }
>
> static int dummy_wakeup(struct usb_gadget *_gadget)
>
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
