libthr cleanup

[Dag-Erling Smørgrav]

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
-- 
Dag-Erling Smørgrav - des at des.no

-------------- next part --------------
A non-text attachment was scrubbed...
Name: libthr.diff
Type: text/x-patch
Size: 52963 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-threads/attachments/20060329/c17aa2e8/libthr.bin