On Thursday, 25 January 2024 11:02:26 CST Arnd Bergmann wrote: > On Wed, Jan 24, 2024, at 23:28, Elizabeth Figura wrote: > > On Wednesday, 24 January 2024 13:52:52 CST Arnd Bergmann wrote: > >> On Wed, Jan 24, 2024, at 19:02, Elizabeth Figura wrote: > >> > That'd be nicer in general. I think there was some documentation that > >> > advised using timespec64 for new ioctl interfaces but it may have been > >> > outdated or misread. > >> > >> It's probably something I wrote. It depends a bit on > >> whether you have an absolute or relative timeout. If > >> the timeout is relative to the current time as I understand > >> it is here, a 64-bit number seems more logical to me. > >> > >> For absolute times, I would usually use a __kernel_timespec, > >> especially if it's CLOCK_REALTIME. In this case you would > >> also need to specify the time domain. > > > > Currently the interface does pass it as an absolute time, with the > > domain implicitly being MONOTONIC. This particular choice comes from > > process/botching-up-ioctls.rst, which is admittedly focused around GPU > > ioctls, but the rationale of having easily restartable ioctls applies > > here too. > > Ok, I was thinking of Documentation/driver-api/ioctl.rst, which > has similar recommendations. > > > (E.g. Wine does play games with signals, so we do want to be able to > > interrupt arbitrary waits with EINTR. The "usual" fast path for ntsync > > waits won't hit that, but we want to have it work.) > > > > On the other hand, if we can pass the timeout as relative, and write it > > back on exit like ppoll() does [assuming that's not proscribed], that > > would presumably be slightly better for performance. > > I've seen arguments go either way between absolute and relative > times, just pick whatever works best for you here. > > > When writing the patch I just picked the recommended option, and didn't > > bother doing any micro-optimizations afterward. > > > > What's the rationale for using timespec for absolute or written-back > > timeouts, instead of dealing in ns directly? I'm afraid it's not > > obvious to me. > > There is no hard rule either way, I mainly didn't like the > indirect pointer to the timespec that you have here. For > traditional unix-style interfaces, a timespec with CLOCK_REALTIME > times usually makes sense since that is what user space is > already using elsewhere, but you probably don't need to > worry about that. In theory, the single u64 CLOCK_REALTIME > nanoseconds have the problem of no longer working after year > 2262, but with CLOCK_MONOTONIC that is not a concern anyway. > > Between embedding a __u64 nanosecond value and embedding > a __kernel_timespec, I would pick whichever avoids converting > a __u64 back into a timespec, as that is an expensive > operation at least on 32-bit code. Makes sense. I'll probably switch to using a relative and written-back u64 then, thanks!
