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!
-
Elizabeth Figura