Improve timeout precision of pthread_cond_timedwait()

[mav]

Summary

This code was not touched when all other user-space sleep paths were switched to sbintime_t and decoupled from hardclock. When it is possible, convert supplied times into sbinuptime to supply directly to msleep_sbt() with C_ABSOLUTE.

Test Plan

Simple test tool shows reachable timeout precision down to few microseconds instead of few milliseconds.

[kib]

sys/kern/kern_umtx.c

724

Can we have the explicit list there, instead of 'default'?

726

You initialize *flags in umtxq_sleep() anyway. I suggest to to *flags |= C_HARDCLOCK there instead (and remove *flags = 0 above), to make the interface somewhat more flexible.

I think e.g. about using C_ABSOLUTE or C_PRECALC there, since right now we do too many conversions for is_abs_real case.

[mav]

sys/kern/kern_umtx.c

724

I suppose just to increase visibility? Because as I was able to find here should appear only 4 different values (see _pthread_condattr_setclock()). Even those values I listed already is a bit overkill.

726

You initialize *flags in umtxq_sleep() anyway. I suggest to to *flags |= C_HARDCLOCK there instead (and remove *flags = 0 above), to make the interface somewhat more flexible.

I don't feel like it would make it better. It would make result depending on previous calls, that while would change nothing is more dirty.

I think e.g. about using C_ABSOLUTE or C_PRECALC there, since right now we do too many conversions for is_abs_real case.

I thought about it briefly, but C_ABSOLUTE require having binuptime, while none of clocks use that. MONOTONIC and UPTIME are close, but require backward conversion from nanoseconds. REALTIME I guess would also require correction on getboottimebin()?

In case of hardclock I haven't actually added any more conversions, just not removed any. If we assume that the only clocks of that kind that may be there are profiles, then relation to hardclock is probably not required (statclock may be even closer for multi-threaded processes) and I could possibly just limit minimal time instead, but unlike the other cases in case of profiles the code has no current binuptime at all, so it can pass only relative sbintime.

[kib]

sys/kern/kern_umtx.c

724

It is it make life easier for anybody reading the code after you, and for code clarity.
Ideally, there would be a default: case with __unreachable() statement.

For instance, it is not clear to me without a lot of code reading which cases go to C_HARDCLOCK. I would expect that at least everything wall-clock related with _PRECISE would, but apparently you did not do it this way.

726

You initialize *flags in umtxq_sleep() anyway. I suggest to to *flags |= C_HARDCLOCK there instead (and remove *flags = 0 above), to make the interface somewhat more flexible.

I don't feel like it would make it better. It would make result depending on previous calls, that while would change nothing is more dirty.

I think e.g. about using C_ABSOLUTE or C_PRECALC there, since right now we do too many conversions for is_abs_real case.

I thought about it briefly, but C_ABSOLUTE require having binuptime, while none of clocks use that. MONOTONIC and UPTIME are close, but require backward conversion from nanoseconds. REALTIME I guess would also require correction on getboottimebin()?

There is already a mechanism to reschedule umtx sleeps on abs time change. This what umtx_abs_time_update() stuff is about, see 9dbdf2a16998ee49011f874b94754e2d046045f3.

In case of hardclock I haven't actually added any more conversions, just not removed any. If we assume that the only clocks of that kind that may be there are profiles, then relation to hardclock is probably not required (statclock may be even closer for multi-threaded processes) and I could possibly just limit minimal time instead, but unlike the other cases in case of profiles the code has no current binuptime at all, so it can pass only relative sbintime.

I am not claiming that you added any new conversions, I am saying that we probably can remove some if this code is better structured.

[mav]

I've rewritten the patch to avoid clock reads when possible, converting most of supplied time values directly into sbinuptime for use with C_ABSOLUTE. For few other (profiling) clocks I don't have good implementation ideas, so left them as-is.

[mav]

Remove unneeded assignment.