The attached patch brings libthr up to WARNS level 2. There is also a small amount of style and whitespace changes mixed in, mostly because I'm so conditioned to style(9) that my fingers sometimes do these things automatically.

Parts of the patch go a little further than level 2, but we're nowhere near level 3 (or antything higher). The major obstacle is the umtx interface, which in my eyes is fundamentally broken.

The _umtx_op() syscall is intended to replace _umtx_lock() and _umtx_unlock(), and support other operations like sleep, wakeup etc.

This is the wrong way to go. If we applied this kind of thinking universally, we'd just define all our system calls as macro wrappers for __syscall().

Beyond these purely philosophical aspects, _umtx_op() seems designed to encourage poor coding practices. It's impossible to use in a type- safe manner: its first argument is supposed to be a struct umtx *, but I don't think there's a single instance in libthr where it is called with an actual struct umtx *. Instead, libthr uses umtx_t, which is defined as long. Sometimes, an umtx_t is passed as the third argument to _umtx_op() as well. Various fields in struct pthread, struct pthread_barrier and struct pthread_cond are declared as umtx_t. Some are used purely as cookies for passing to _umtx_op(); some are used as counters or state variables.

It's a miracle libthr works at all. The kernel expects a pointer to a struct umtx; instead, it gets (mostly) a pointer to long. Luckily, struct umtx (which contains a single pointer) is the same size as long on all our platforms.

_umtx_op() needs to be split into four system calls:

int _umtx_lock_timeout(struct umtx *mtx, const struct timespec * __restrict timeout); int _umtx_unlock(struct umtx *mtx); int _umtx_wait_timeout(const void *cookie, struct umtx *mtx, const struct timespec * __restrict timeout); int _umtx_wake(const void *cookie);

_umtx_unlock() already exists with the correct semantics; the rest are new. Note that we can't just add a struct timespec to _umtx_lock(), as that would break the upgrade path from RELENG_5; hence the _timeout suffix.

I'm not sure the current implementation of UMTX_OP_WAIT / UMTX_OP_WAKE in the kernel is correct. Normally, a wait primitive (like msleep(), pthread_cond_wait() etc.) takes a cookie and a mutex, and unlocks the mutex while it's sleeping on the cookie. It looks like the umtx code originally worked like this, but I'm not sure it does anymore; it's hard to unravel, partly because I haven't quite figured out the queues yet and partly because I haven't had breakfast.

DES