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 pingbo.wen@linaro.org Cc: Y2038 y2038@lists.linaro.org
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)