Commit d955dfb0 authored by Matthew Dempsky's avatar Matthew Dempsky Committed by Ian Lance Taylor

runtime: cleanup openbsd semasleep implementation

The previous implementation had several subtle issues.  It's not
clear if any of these could actually be causing the flakiness
problems on openbsd/386, but fixing them should only help.

1. thrsleep() is implemented internally as unlock, then test *abort
(if abort != nil), then tsleep().  Under the current code, that makes
it theoretically possible that semasleep()/thrsleep() could release
waitsemalock, then a racing semawakeup() could acquire the lock,
increment waitsemacount, and call thrwakeup()/wakeup() before
thrsleep() reaches tsleep().  (In practice, OpenBSD's big kernel lock
seems unlikely to let this actually happen.)

The proper way to avoid this is to pass &waitsemacount as the abort
pointer to thrsleep so thrsleep knows to re-check it before going to
sleep, and to wakeup if it's non-zero.  Then we avoid any races.
(I actually suspect openbsd's sema{sleep,wakeup}() could be further
simplified using cas/xadd instead of locks, but I don't want to be
more intrusive than necessary so late in the 1.4 release cycle.)

2. semasleep() takes a relative sleep duration, but thrsleep() needs
an absolute sleep deadline.  Instead of recomputing the deadline each
iteration, compute it once up front and use (*Timespec)(nil) to signify
no deadline.  Ensures we retry properly if there's a spurious wakeup.

3. Instead of assuming if thrsleep() woke up and waitsemacount wasn't
available that we must have hit the deadline, check that the system
call returned EWOULDBLOCK.

4. Instead of assuming that 64-bit systems are little-endian, compute
timediv() using a temporary int32 nsec and then assign it to tv_nsec.

LGTM=iant
R=jsing, iant
CC=golang-codereviews
https://golang.org/cl/137960043
parent 9f012e10
......@@ -12,6 +12,8 @@
enum
{
ESRCH = 3,
EAGAIN = 35,
EWOULDBLOCK = EAGAIN,
ENOTSUP = 91,
// From OpenBSD's sys/time.h
......@@ -65,32 +67,24 @@ runtime·semacreate(void)
int32
runtime·semasleep(int64 ns)
{
Timespec ts;
// spin-mutex lock
while(runtime·xchg(&g->m->waitsemalock, 1))
runtime·osyield();
Timespec ts, *tsp = nil;
// Compute sleep deadline.
if(ns >= 0) {
int32 nsec;
ns += runtime·nanotime();
ts.tv_sec = runtime·timediv(ns, 1000000000, &nsec);
ts.tv_nsec = nsec; // tv_nsec is int64 on amd64
tsp = &ts;
}
for(;;) {
// lock held
if(g->m->waitsemacount == 0) {
// sleep until semaphore != 0 or timeout.
// thrsleep unlocks m->waitsemalock.
if(ns < 0)
runtime·thrsleep(&g->m->waitsemacount, 0, nil, &g->m->waitsemalock, nil);
else {
ns += runtime·nanotime();
// NOTE: tv_nsec is int64 on amd64, so this assumes a little-endian system.
ts.tv_nsec = 0;
ts.tv_sec = runtime·timediv(ns, 1000000000, (int32*)&ts.tv_nsec);
runtime·thrsleep(&g->m->waitsemacount, CLOCK_MONOTONIC, &ts, &g->m->waitsemalock, nil);
}
// reacquire lock
while(runtime·xchg(&g->m->waitsemalock, 1))
runtime·osyield();
}
int32 ret;
// spin-mutex lock
while(runtime·xchg(&g->m->waitsemalock, 1))
runtime·osyield();
// lock held (again)
if(g->m->waitsemacount != 0) {
// semaphore is available.
g->m->waitsemacount--;
......@@ -99,17 +93,12 @@ runtime·semasleep(int64 ns)
return 0; // semaphore acquired
}
// semaphore not available.
// if there is a timeout, stop now.
// otherwise keep trying.
if(ns >= 0)
break;
// sleep until semaphore != 0 or timeout.
// thrsleep unlocks m->waitsemalock.
ret = runtime·thrsleep(&g->m->waitsemacount, CLOCK_MONOTONIC, tsp, &g->m->waitsemalock, (int32 *)&g->m->waitsemacount);
if(ret == EWOULDBLOCK)
return -1;
}
// lock held but giving up
// spin-mutex unlock
runtime·atomicstore(&g->m->waitsemalock, 0);
return -1;
}
static void badsemawakeup(void);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment