[PATCH 2/4] Fix futex timeouts and handle futex error codes.
[Thread Prev] | [Thread Next]
- Subject: [PATCH 2/4] Fix futex timeouts and handle futex error codes.
- From: iriri <iri@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
- Reply-to: myrddin-dev@xxxxxxxxxxxxxx
- Date: Sat, 18 Aug 2018 16:17:42 -0700
- To: "myrddin-dev" <myrddin-dev@xxxxxxxxxxxxxx>
---
lib/sys/sys+freebsd-x64.myr | 9 ++-
lib/sys/sys+linux-x64.myr | 6 +-
lib/thread/condvar+freebsd.myr | 15 ++--
lib/thread/condvar+linux.myr | 27 +++----
lib/thread/condvar+openbsd:6.2.myr | 27 +++----
lib/thread/condvar+osx.myr | 15 ++--
lib/thread/condvar.myr | 36 ++++-----
lib/thread/futex+freebsd.myr | 56 +++++++++-----
lib/thread/futex+linux.myr | 59 ++++++++++++---
lib/thread/futex+openbsd:6.2.myr | 66 ++++++++++++++--
lib/thread/futex+osx.myr | 75 ++++++++++++-------
lib/thread/mutex+futex.myr | 11 ++-
lib/thread/mutex+plan9.myr | 2 -
lib/thread/mutex.myr | 2 -
lib/thread/sem+futex.myr | 3 +-
lib/thread/sem+plan9.myr | 1 -
lib/thread/waitgrp+futex.myr | 3 +-
support/syscall-gen/specials+freebsd-x64.frag | 1 +
support/syscall-gen/specials+linux-x64.frag | 4 +-
support/syscall-gen/types+freebsd-x64.frag | 7 ++
support/syscall-gen/types+linux-x64.frag | 2 +
21 files changed, 268 insertions(+), 159 deletions(-)
diff --git a/lib/sys/sys+freebsd-x64.myr b/lib/sys/sys+freebsd-x64.myr
index e1cfb692..b89e99b9 100644
--- a/lib/sys/sys+freebsd-x64.myr
+++ b/lib/sys/sys+freebsd-x64.myr
@@ -582,10 +582,10 @@ pkg sys =
;;
const Umtxabstime = 1
- type _umtx_time = struct
- _timeout : timespec
- _flags : uint32
- _clockid : uint32
+ type umtx_time = struct
+ timeout : timespec
+ flags : uint32
+ clockid : uint32
;;
/* open options */
@@ -1192,6 +1192,7 @@ pkg sys =
const getsockopt : (sock : fd, lev : sockproto, opt : sockopt, val : void#, len : size# -> int)
const munmap : (addr:byte#, len:size -> int64)
const mmap : (addr:byte#, len:size, prot:mprot, flags:mopt, fd:fd, off:off -> byte#)
+ const clockid : (clk : clock -> uint64)
const clock_getres : (clk : clock, ts : timespec# -> int32)
const clock_gettime : (clk : clock, ts : timespec# -> int32)
const clock_settime : (clk : clock, ts : timespec# -> int32)
diff --git a/lib/sys/sys+linux-x64.myr b/lib/sys/sys+linux-x64.myr
index 1cebb123..61bb8b2d 100644
--- a/lib/sys/sys+linux-x64.myr
+++ b/lib/sys/sys+linux-x64.myr
@@ -563,6 +563,8 @@ pkg sys =
const Futexpriv : futexop = 128
const Futexclockrt : futexop = 256
+ const Futexbitsetmatchany : int32 = -1
+
/* poll events : posix */
const Pollin : pollevt = 0x001 /* There is data to read. */
const Pollpri : pollevt = 0x002 /* There is urgent data to read. */
@@ -1273,7 +1275,7 @@ pkg sys =
const dup : (fd : fd -> fd)
const dup2 : (src : fd, dst : fd -> fd)
const futex : (uaddr : int32#, op : futexop, val : int32, \
- timeout : timespec#, uaddr2 : int32#, val3 : int32 -> int64)
+ timeout : timespec#, uaddr2 : int32#, val3 : int32 -> int)
const semctl : (semid : int, semnum : int, cmd : int, arg : void# -> int)
const epollcreate : (flg : epollflags -> fd) /* actually epoll_create1 */
const epollctl : (epfd : fd, op : int, fd : fd, evt : epollevt# -> int)
@@ -1692,7 +1694,7 @@ pkg sys =
/* threading */
const futex = {uaddr, op, val, timeout, uaddr2, val3
- -> syscall(Sysfutex, a(uaddr), a(op), a(val), a(timeout), a(uaddr2), a(val3))
+ -> (syscall(Sysfutex, a(uaddr), a(op), a(val), a(timeout), a(uaddr2), a(val3)) : int)
}
const semctl = {semid, semnum, cmd, arg
-> (syscall(Syssemctl, a(semnum), a(cmd), a(arg)) : int)
diff --git a/lib/thread/condvar+freebsd.myr b/lib/thread/condvar+freebsd.myr
index 002757ae..7b2e77c3 100644
--- a/lib/thread/condvar+freebsd.myr
+++ b/lib/thread/condvar+freebsd.myr
@@ -1,7 +1,6 @@
use std
use "atomic"
-use "common"
use "mutex"
use "futex"
@@ -22,19 +21,17 @@ const mkcond = {mtx
}
const condwait = {cond
- var seq
- var mtx
+ var mtx = cond._mtx
+ var seq = xget(&cond._seq)
- mtx = cond._mtx
- seq = cond._seq
mtxunlock(mtx)
+ ftxwait(&cond._seq, seq, -1)
/*
- FIXME?: `ftxwait` can be interrupted but `condwait` should always be
- done in a loop anyway.
+ In the event of a broadcast, we aren't requeueing waiters from the
+ condvar's futex to the mutex's futex so we can just reacquire the mutex
+ normally.
*/
- ftxwait(&cond._seq, seq, Zptr)
-
mtxlock(mtx)
}
diff --git a/lib/thread/condvar+linux.myr b/lib/thread/condvar+linux.myr
index e1a9e100..8d3f417b 100644
--- a/lib/thread/condvar+linux.myr
+++ b/lib/thread/condvar+linux.myr
@@ -3,12 +3,13 @@ use sys
use "atomic"
use "common"
+use "futex"
use "mutex"
pkg thread =
type cond = struct
_mtx : mutex#
- _seq : int32
+ _seq : ftxtag
;;
const mkcond : (mtx : mutex# -> cond)
@@ -22,30 +23,24 @@ const mkcond = {mtx
}
const condwait = {cond
- var seq
- var mtx
+ var mtx = cond._mtx
+ var seq = xget(&cond._seq)
- mtx = cond._mtx
- seq = cond._seq
mtxunlock(mtx)
+ ftxwait(&cond._seq, seq, -1)
/*
- FIXME?: `futex` can be interrupted but `condwait` should always be done
- in a loop anyway.
- */
- sys.futex(&cond._seq, sys.Futexwait | sys.Futexpriv, seq, Zptr, Zptr, 0)
-
- /*
- We need to atomically set the mutex to contended. This allows us to
- pass responsibility for waking up the potential other waiters on to the
- unlocker of the mutex.
+ In the event of a broadcast, we need to atomically set the mutex to
+ contended. This allows us to pass responsibility for waking up the
+ potential other waiters from the requeue operation on to the unlocker
+ of the mutex.
*/
mtxcontended(mtx)
}
const condsignal = {cond : cond#
xadd(&cond._seq, 1)
- sys.futex(&cond._seq, sys.Futexwake | sys.Futexpriv, 1, Zptr, Zptr, 0)
+ ftxwake(&cond._seq)
}
const condbroadcast = {cond : cond#
@@ -55,7 +50,7 @@ const condbroadcast = {cond : cond#
used for the number of threads to move, and is not ignored when
requeueing
*/
- sys.futex(&cond._seq, sys.Futexrequeue | sys.Futexpriv,
+ sys.futex((&cond._seq : int32#), sys.Futexrequeue | sys.Futexpriv,
1, (0x7fffffff : sys.timespec#),
(&cond._mtx._state : int32#), 0)
}
diff --git a/lib/thread/condvar+openbsd:6.2.myr b/lib/thread/condvar+openbsd:6.2.myr
index 29dbb35f..345a3b17 100644
--- a/lib/thread/condvar+openbsd:6.2.myr
+++ b/lib/thread/condvar+openbsd:6.2.myr
@@ -3,12 +3,13 @@ use sys
use "atomic"
use "common"
+use "futex"
use "mutex"
pkg thread =
type cond = struct
_mtx : mutex#
- _seq : uint32
+ _seq : ftxtag
;;
const mkcond : (mtx : mutex# -> cond)
@@ -22,35 +23,29 @@ const mkcond = {mtx
}
const condwait = {cond
- var seq : uint32
- var mtx
+ var mtx = cond._mtx
+ var seq = xget(&cond._seq)
- mtx = cond._mtx
- seq = cond._seq
mtxunlock(mtx)
+ ftxwait(&cond._seq, seq, -1)
/*
- FIXME?: `futex` can be interrupted but `condwait` should always be done
- in a loop anyway.
- */
- sys.futex(&cond._seq, sys.Futexwait, (seq : int), Zptr, Zptr)
-
- /*
- We need to atomically set the mutex to contended. This allows us to
- pass responsibility for waking up the potential other waiters on to the
- unlocker of the mutex.
+ In the event of a broadcast, we need to atomically set the mutex to
+ contended. This allows us to pass responsibility for waking up the
+ potential other waiters from the requeue operation on to the unlocker
+ of the mutex.
*/
mtxcontended(mtx)
}
const condsignal = {cond : cond#
xadd(&cond._seq, 1)
- sys.futex(&cond._seq, sys.Futexwake, 1, Zptr, Zptr)
+ ftxwake(&cond._seq)
}
const condbroadcast = {cond : cond#
xadd(&cond._seq, 1)
- sys.futex(&cond._seq, sys.Futexrequeue, 1,
+ sys.futex((&cond._seq : uint32#), sys.Futexrequeue, 1,
(0x7fffffff : sys.timespec#),
(&cond._mtx._state : uint32#))
}
diff --git a/lib/thread/condvar+osx.myr b/lib/thread/condvar+osx.myr
index d74c321f..0f1b1ff4 100644
--- a/lib/thread/condvar+osx.myr
+++ b/lib/thread/condvar+osx.myr
@@ -1,7 +1,6 @@
use std
use "atomic"
-use "common"
use "mutex"
use "futex"
@@ -22,19 +21,17 @@ const mkcond = {mtx
}
const condwait = {cond
- var seq
- var mtx
+ var mtx = cond._mtx
+ var seq = xget(&cond._seq)
- mtx = cond._mtx
- seq = cond._seq
mtxunlock(mtx)
+ ftxwait(&cond._seq, seq, -1)
/*
- FIXME?: `ftxwait` can be interrupted but `condwait` should always be
- done in a loop anyway.
+ In the event of a broadcast, we aren't requeueing waiters from the
+ condvar's futex to the mutex's futex so we can just reacquire the mutex
+ normally.
*/
- ftxwait(&cond._seq, seq, Zptr)
-
mtxlock(mtx)
}
diff --git a/lib/thread/condvar.myr b/lib/thread/condvar.myr
index 4fde7073..f843f843 100644
--- a/lib/thread/condvar.myr
+++ b/lib/thread/condvar.myr
@@ -8,7 +8,8 @@ use "sem"
pkg thread =
type cond = struct
_mtx : mutex#
- _waitq : condwaiter#
+ _head : condwaiter#
+ _tail : condwaiter#
_lock : mutex
;;
@@ -18,15 +19,8 @@ pkg thread =
const condbroadcast : (cond : cond# -> void)
;;
-/*
-The waitqueue is a doubly-linked list because we'll need to remove waiters from
-anywhere in the list when we add timeout support.
-
-`cond._waitq.prev` is the tail of the queue.
-*/
type condwaiter = struct
next : condwaiter#
- prev : condwaiter#
sem : sem
;;
@@ -40,14 +34,9 @@ const condwait = {cond
var waiter = std.mk([.sem = mksem(0)])
mtxlock(lock)
- match cond._waitq
- | Zptr:
- waiter.prev = waiter
- cond._waitq = waiter
- | q:
- waiter.prev = q.prev
- waiter.prev.next = waiter
- q.prev = waiter
+ match cond._tail
+ | Zptr: cond._head = cond._tail = waiter
+ | tail: cond._tail = tail.next = waiter
;;
mtxunlock(lock)
@@ -61,13 +50,13 @@ const condsignal = {cond
var lock = &cond._lock
mtxlock(lock)
- var head = cond._waitq
+ var head = cond._head
if head != Zptr
- if head.next != Zptr
- head.next.prev = head.prev
- ;;
- cond._waitq = head.next
+ cond._head = head.next
sempost(&head.sem)
+ if cond._head == Zptr
+ cond._tail = Zptr
+ ;;
;;
mtxunlock(lock)
}
@@ -81,9 +70,10 @@ const condbroadcast = {cond
var head = Zptr
mtxlock(lock)
- while (head = cond._waitq) != Zptr
- cond._waitq = head.next
+ while (head = cond._head) != Zptr
+ cond._head = head.next
sempost(&head.sem)
;;
+ cond._tail = Zptr
mtxunlock(lock)
}
diff --git a/lib/thread/futex+freebsd.myr b/lib/thread/futex+freebsd.myr
index ea5f28d8..c011bf94 100644
--- a/lib/thread/futex+freebsd.myr
+++ b/lib/thread/futex+freebsd.myr
@@ -1,3 +1,4 @@
+use std
use sys
use "atomic"
@@ -7,34 +8,53 @@ pkg thread =
type ftxtag = uint32
impl atomic ftxtag
- const ftxwait : (uaddr : ftxtag#, val : ftxtag, timeout : sys.timespec# -> int)
- const ftxwake : (uaddr : ftxtag# -> int)
- const ftxwakeall : (uaddr : ftxtag# -> int)
+ const ftxwait : (uaddr : ftxtag#, val : ftxtag, tmout : std.time -> sys.errno)
+ const ftxwake : (uaddr : ftxtag# -> void)
+ const ftxwakeall : (uaddr : ftxtag# -> void)
;;
-const ftxwait = {uaddr, val, timeout
- if timeout == Zptr
- -> sys.umtx_op((uaddr : void#), sys.Umtxwaituintpriv, (val : uint64), Zptr, Zptr)
+const ftxwait = {uaddr, val, tmout
+ var ut : sys.umtx_time, utp, utsize, rc
+
+ if tmout < 0
+ utp = Zptr
+ utsize = Zptr
+ else
+ var t = (tmout : int64)
+ std.assert(sys.clock_gettime(`sys.Clockmonotonic, &ut.timeout) == 0,
+ "error: clock_gettime returned -1\n")
+ ut.timeout.nsec += (t % 1_000_000) * 1000
+ ut.timeout.sec += (ut.timeout.nsec / 1_000_000_000) + (t / 1_000_000)
+ ut.timeout.nsec %= 1_000_000_000
+ ut.flags = (sys.Umtxabstime : uint32)
+ ut.clockid = (sys.clockid(`sys.Clockmonotonic) : uint32)
+ utp = (&ut : void#)
+ utsize = (sizeof(sys.umtx_time) : void#)
+ ;;
+
+ while (rc = (sys.umtx_op((uaddr : void#),
+ sys.Umtxwaituintpriv,
+ (val : uint64),
+ utsize,
+ utp) : sys.errno)) == sys.Eintr
;;
- var ut : sys._umtx_time = [
- ._timeout = timeout#,
- ._flags = (sys.Umtxabstime : uint32),
- ._clockid = 1, /* CLOCK_MONOTONIC. Not exported from sys. */
- ]
- -> sys.umtx_op((uaddr : void#),
- sys.Umtxwaituintpriv,
- (val : uint64),
- (sizeof(sys._umtx_time) : void#),
- (&ut : void#))
+ match rc
+ | 0: -> 0
+ | sys.Eagain: -> sys.Eagain
+ | sys.Etimedout: -> sys.Etimedout
+ | err:
+ std.fput(2, "error: umtx_op returned {}\n", err)
+ std.suicide()
+ ;;
}
const ftxwake = {uaddr
- -> sys.umtx_op((uaddr : void#), sys.Umtxwakepriv, 1, Zptr, Zptr)
+ sys.umtx_op((uaddr : void#), sys.Umtxwakepriv, 1, Zptr, Zptr)
}
const ftxwakeall = {uaddr
- -> sys.umtx_op((uaddr : void#), sys.Umtxwakepriv, 0x7fffffff, Zptr, Zptr)
+ sys.umtx_op((uaddr : void#), sys.Umtxwakepriv, 0x7fffffff, Zptr, Zptr)
}
impl atomic ftxtag =
diff --git a/lib/thread/futex+linux.myr b/lib/thread/futex+linux.myr
index c5dbf062..c182644c 100644
--- a/lib/thread/futex+linux.myr
+++ b/lib/thread/futex+linux.myr
@@ -1,3 +1,4 @@
+use std
use sys
use "atomic"
@@ -7,26 +8,60 @@ pkg thread =
type ftxtag = uint32
impl atomic ftxtag
- const ftxwait : (uaddr : ftxtag#, val : ftxtag, timeout : sys.timespec# -> int)
- const ftxwake : (uaddr : ftxtag# -> int)
- const ftxwakeall : (uaddr : ftxtag# -> int)
+ const ftxwait : (uaddr : ftxtag#, val : ftxtag, tmout : std.time -> sys.errno)
+ const ftxwake : (uaddr : ftxtag# -> void)
+ const ftxwakeall : (uaddr : ftxtag# -> void)
;;
-const ftxwait = {uaddr, val, timeout
- -> (sys.futex((uaddr : int32#),
- sys.Futexwait | sys.Futexpriv,
- (val : int32),
- timeout,
- Zptr,
- 0) : int)
+const ftxwait = {uaddr, val, tmout
+ var rc
+
+ if tmout < 0
+ while (rc = (sys.futex((uaddr : int32#),
+ sys.Futexwait | sys.Futexpriv,
+ (val : int32),
+ Zptr,
+ Zptr,
+ 0) : sys.errno)) == sys.Eintr
+ ;;
+ else
+ var t = (tmout : int64)
+ var ts
+ std.assert(sys.clock_gettime(`sys.Clockmonotonic, &ts) == 0,
+ "error: clock_gettime returned -1\n")
+ ts.nsec += (t % 1_000_000) * 1000
+ ts.sec += (ts.nsec / 1_000_000_000) + (t / 1_000_000)
+ ts.nsec %= 1_000_000_000
+
+ /*
+ Futexwaitbitset + Futexbitsetmatchany causes the timeout to be
+ treated as absolute rather than relative.
+ */
+ while (rc = (sys.futex((uaddr : int32#),
+ sys.Futexwaitbitset | sys.Futexpriv,
+ (val : int32),
+ &ts,
+ Zptr,
+ sys.Futexbitsetmatchany) : sys.errno)) == sys.Eintr
+ ;;
+ ;;
+
+ match rc
+ | 0: -> 0
+ | sys.Eagain: -> sys.Eagain
+ | sys.Etimedout: -> sys.Etimedout
+ | err:
+ std.fput(2, "error: futex returned {}\n", err)
+ std.suicide()
+ ;;
}
const ftxwake = {uaddr
- -> (sys.futex((uaddr : int32#), sys.Futexwake | sys.Futexpriv, 1, Zptr, Zptr, 0) : int)
+ sys.futex((uaddr : int32#), sys.Futexwake | sys.Futexpriv, 1, Zptr, Zptr, 0)
}
const ftxwakeall = {uaddr
- -> (sys.futex((uaddr : int32#), sys.Futexwake | sys.Futexpriv, 0x7fffffff, Zptr, Zptr, 0) : int)
+ sys.futex((uaddr : int32#), sys.Futexwake | sys.Futexpriv, 0x7fffffff, Zptr, Zptr, 0)
}
impl atomic ftxtag =
diff --git a/lib/thread/futex+openbsd:6.2.myr b/lib/thread/futex+openbsd:6.2.myr
index e3c9c413..5778e288 100644
--- a/lib/thread/futex+openbsd:6.2.myr
+++ b/lib/thread/futex+openbsd:6.2.myr
@@ -1,3 +1,4 @@
+use std
use sys
use "atomic"
@@ -7,21 +8,72 @@ pkg thread =
type ftxtag = uint32
impl atomic ftxtag
- const ftxwait : (uaddr : ftxtag#, val : ftxtag, timeout : sys.timespec# -> int)
- const ftxwake : (uaddr : ftxtag# -> int)
- const ftxwakeall : (uaddr : ftxtag# -> int)
+ const ftxwait : (uaddr : ftxtag#, val : ftxtag, tmout : std.time -> sys.errno)
+ const ftxwake : (uaddr : ftxtag# -> void)
+ const ftxwakeall : (uaddr : ftxtag# -> void)
;;
-const ftxwait = {uaddr, val, timeout
- -> sys.futex((uaddr : uint32#), sys.Futexwait, (val : int), timeout, Zptr)
+const ftxwait = {uaddr, val, tmout
+ var rc
+
+ if tmout < 0
+ while (rc = (sys.futex((uaddr : uint32#),
+ sys.Futexwait,
+ (val : int),
+ Zptr,
+ Zptr) : sys.errno)) == sys.Eintr
+ ;;
+ else
+ var t = (tmout : int64)
+ var start
+ std.assert(sys.clock_gettime(`sys.Clockmonotonic, &start) == 0,
+ "error: clock_gettime returned -1\n")
+ var ts = [
+ .sec = t / 1_000_000
+ .nsec = (t % 1_000_000) * 1000
+ ]
+
+ while (rc = (sys.futex((uaddr : uint32#),
+ sys.Futexwait,
+ (val : int),
+ &ts,
+ Zptr) : sys.errno)) == sys.Eintr
+ var now
+ std.assert(sys.clock_gettime(`sys.Clockmonotonic, &now) == 0,
+ "error: clock_gettime returned -1\n")
+ var t1 = t - ((now.sec - start.sec) * 1_000_000)
+ var nsec = now.nsec - start.nsec
+ if nsec >= 0
+ t1 -= nsec / 1000
+ else
+ t1 -= (1_000_000_000 + nsec) / 1000
+ ;;
+
+ if t1 > t
+ -> sys.Etimedout
+ ;;
+ ts.sec = t1 / 1_000_000
+ ts.nsec = (t1 % 1_000_000) * 1000
+ t = t1
+ ;;
+ ;;
+
+ match rc
+ | 0: -> 0
+ | sys.Eagain: -> sys.Eagain
+ | sys.Etimedout: -> sys.Etimedout
+ | err:
+ std.fput(2, "error: futex returned {}\n", err)
+ std.suicide()
+ ;;
}
const ftxwake = {uaddr
- -> sys.futex((uaddr : uint32#), sys.Futexwake, 1, Zptr, Zptr)
+ sys.futex((uaddr : uint32#), sys.Futexwake, 1, Zptr, Zptr)
}
const ftxwakeall = {uaddr
- -> sys.futex((uaddr : uint32#), sys.Futexwake, 0x7fffffff, Zptr, Zptr)
+ sys.futex((uaddr : uint32#), sys.Futexwake, 0x7fffffff, Zptr, Zptr)
}
impl atomic ftxtag =
diff --git a/lib/thread/futex+osx.myr b/lib/thread/futex+osx.myr
index 17478703..ea93fda7 100644
--- a/lib/thread/futex+osx.myr
+++ b/lib/thread/futex+osx.myr
@@ -8,49 +8,72 @@ pkg thread =
type ftxtag = uint64
impl atomic ftxtag
- const ftxwait : (uaddr : ftxtag#, val : ftxtag, timeout : sys.timespec# -> int)
- const ftxwake : (uaddr : ftxtag# -> int)
- const ftxwakeall : (uaddr : ftxtag# -> int)
+ const ftxwait : (uaddr : ftxtag#, val : ftxtag, tmout : std.time -> sys.errno)
+ const ftxwake : (uaddr : ftxtag# -> void)
+ const ftxwakeall : (uaddr : ftxtag# -> void)
;;
/*
* The ulock_ functions are undocumented but the relevant source can be found at
* https://github.com/apple/darwin-xnu/blob/0a798f6738bc1db01281fc08ae024145e84df927/bsd/kern/sys_ulock.c
*/
-const ftxwait = {uaddr, val, timeout
- if timeout == Zptr
- -> sys.ulock_wait(sys.Ulockcompareandwait, (uaddr : uint64#), (val : uint64), 0)
- ;;
+const ftxwait = {uaddr, val, tmout
+ var rc
- var ts
- var err = sys.clock_gettime(`sys.Clockmonotonic, &ts)
- std.assert(err == 0, "error: clock_gettime returned {}\n", err)
- if timeout.sec < ts.sec
- -> (std.Etimedout : int)
- ;;
+ if tmout < 0
+ while (rc = (sys.ulock_wait(sys.Ulockcompareandwait,
+ (uaddr : uint64#),
+ (val : uint64),
+ 0) : sys.errno)) == sys.Eintr
+ ;;
+ else
+ var start, t
+ std.assert(tmout <= 0xffffffff, "error: maximum os x futex timeout exceeded\n")
+ std.assert(sys.clock_gettime(`sys.Clockmonotonic, &start) == 0,
+ "error: clock_gettime returned -1\n")
+ t = (tmout : uint32)
+
+ while (rc = (sys.ulock_wait(sys.Ulockcompareandwait,
+ (uaddr : uint64#),
+ (val : uint64),
+ t) : sys.errno)) == sys.Eintr
+ var now
+ std.assert(sys.clock_gettime(`sys.Clockmonotonic, &now) == 0,
+ "error: clock_gettime returned -1\n")
+ var t1 = t - (((now.sec - start.sec) * 1_000_000) : uint32)
+ var nsec = now.nsec - start.nsec
+ if nsec >= 0
+ t1 -= (nsec / 1000 : uint32)
+ else
+ t1 -= ((1_000_000_000 + nsec) / 1000 : uint32)
+ ;;
- var usec = 0
- var sec = (timeout.sec - ts.sec) * 1000
- std.assert(sec <= 0xffffffff, "error: maximum futex timeout exceeded\n")
- usec = (sec : uint32)
- if timeout.nsec > ts.nsec
- var nsec = (timeout.nsec - ts.nsec) / 1000
- std.assert(usec + nsec > usec, "error: maximum futex timeout exceeded\n")
- usec += nsec
+ if t1 > t
+ -> sys.Etimedout
+ ;;
+ t = t1
+ ;;
;;
- if usec == 0
- -> (std.Etimedout : int)
+ match rc
+ | 0: -> 0
+ | sys.Eagain: -> sys.Eagain
+ | sys.Etimedout: -> sys.Etimedout
+ | err:
+ if err > 0
+ -> 0
+ ;;
+ std.fput(2, "error: ulock_wait returned {}\n", err)
+ std.suicide()
;;
- -> sys.ulock_wait(sys.Ulockcompareandwait, (uaddr : uint64#), (val : uint64), usec)
}
const ftxwake = {uaddr
- -> sys.ulock_wake(sys.Ulockcompareandwait, (uaddr : uint64#), 0)
+ sys.ulock_wake(sys.Ulockcompareandwait, (uaddr : uint64#), 0)
}
const ftxwakeall = {uaddr
- -> sys.ulock_wake(sys.Ulockcompareandwait | sys.Ulockulfwakeall, (uaddr : uint64#), 0)
+ sys.ulock_wake(sys.Ulockcompareandwait | sys.Ulockulfwakeall, (uaddr : uint64#), 0)
}
impl atomic ftxtag =
diff --git a/lib/thread/mutex+futex.myr b/lib/thread/mutex+futex.myr
index a6453fa9..50e8406d 100644
--- a/lib/thread/mutex+futex.myr
+++ b/lib/thread/mutex+futex.myr
@@ -1,5 +1,4 @@
use "atomic"
-use "common"
use "futex"
pkg thread =
@@ -28,12 +27,12 @@ const mkmtx = {
const mtxlock = {mtx
var c
- /*
+ /*
Uncontended case: we get an unlocked mutex, and we lock it.
*/
- c = Locked
+ c = Locked
for var i = 0; i < nspin; i++
- c = xcas(&mtx._state, Unlocked, Locked)
+ c = xcas(&mtx._state, Unlocked, Locked)
if c == Unlocked
-> void
;;
@@ -49,7 +48,7 @@ const mtxlock = {mtx
;;
while c != Unlocked
- ftxwait(&mtx._state, Contended, Zptr)
+ ftxwait(&mtx._state, Contended, -1)
c = xchg(&mtx._state, Contended)
;;
}
@@ -74,6 +73,6 @@ const mtxunlock = {mtx
const mtxcontended = {mtx
while xchg(&mtx._state, Contended) != Unlocked
- ftxwait(&mtx._state, Contended, Zptr)
+ ftxwait(&mtx._state, Contended, -1)
;;
}
diff --git a/lib/thread/mutex+plan9.myr b/lib/thread/mutex+plan9.myr
index e2e1207f..67c93bfb 100644
--- a/lib/thread/mutex+plan9.myr
+++ b/lib/thread/mutex+plan9.myr
@@ -1,9 +1,7 @@
use std
use sys
-
use "atomic"
-use "common"
pkg thread =
type mutex = struct
diff --git a/lib/thread/mutex.myr b/lib/thread/mutex.myr
index 5b905749..b37f2fb3 100644
--- a/lib/thread/mutex.myr
+++ b/lib/thread/mutex.myr
@@ -1,9 +1,7 @@
use std
use sys
-
use "atomic"
-use "common"
pkg thread =
type mutex = struct
diff --git a/lib/thread/sem+futex.myr b/lib/thread/sem+futex.myr
index d79bd41b..542586be 100644
--- a/lib/thread/sem+futex.myr
+++ b/lib/thread/sem+futex.myr
@@ -1,7 +1,6 @@
use std
use "atomic"
-use "common"
use "futex"
pkg thread =
@@ -28,7 +27,7 @@ const semwait = {s
-> void
;;
;;
- ftxwait(&s._val, v, Zptr)
+ ftxwait(&s._val, v, -1)
;;
}
diff --git a/lib/thread/sem+plan9.myr b/lib/thread/sem+plan9.myr
index 221a8056..449d7ceb 100644
--- a/lib/thread/sem+plan9.myr
+++ b/lib/thread/sem+plan9.myr
@@ -2,7 +2,6 @@ use std
use sys
use "atomic"
-use "common"
pkg thread =
type sem = struct
diff --git a/lib/thread/waitgrp+futex.myr b/lib/thread/waitgrp+futex.myr
index 7fbd5eb6..701d8f04 100644
--- a/lib/thread/waitgrp+futex.myr
+++ b/lib/thread/waitgrp+futex.myr
@@ -1,7 +1,6 @@
use std
use "atomic"
-use "common"
use "futex"
pkg thread =
@@ -22,7 +21,7 @@ const wgwait = {w
var v = 0
while (v = xget(&w._val)) != 0
- ftxwait(&w._val, v, Zptr)
+ ftxwait(&w._val, v, -1)
;;
}
diff --git a/support/syscall-gen/specials+freebsd-x64.frag b/support/syscall-gen/specials+freebsd-x64.frag
index 82b3c31b..de5cfa4a 100644
--- a/support/syscall-gen/specials+freebsd-x64.frag
+++ b/support/syscall-gen/specials+freebsd-x64.frag
@@ -65,6 +65,7 @@
const mmap : (addr:byte#, len:size, prot:mprot, flags:mopt, fd:fd, off:off -> byte#)
/* time - doublecheck if this is right */
+ const clockid : (clk : clock -> uint64)
const clock_getres : (clk : clock, ts : timespec# -> int32)
const clock_gettime : (clk : clock, ts : timespec# -> int32)
const clock_settime : (clk : clock, ts : timespec# -> int32)
diff --git a/support/syscall-gen/specials+linux-x64.frag b/support/syscall-gen/specials+linux-x64.frag
index de2ce273..6f8b8063 100644
--- a/support/syscall-gen/specials+linux-x64.frag
+++ b/support/syscall-gen/specials+linux-x64.frag
@@ -62,7 +62,7 @@ const dup2 : (src : fd, dst : fd -> fd)
/* threading */
const futex : (uaddr : int32#, op : futexop, val : int32, \
- timeout : timespec#, uaddr2 : int32#, val3 : int32 -> int64)
+ timeout : timespec#, uaddr2 : int32#, val3 : int32 -> int)
const semctl : (semid : int, semnum : int, cmd : int, arg : void# -> int)
/* polling */
@@ -205,7 +205,7 @@ const sigprocmask = {sig, act, oact; -> (syscall(Sysrt_sigprocmask, a(sig), a(ac
/* threading */
const futex = {uaddr, op, val, timeout, uaddr2, val3
- -> syscall(Sysfutex, a(uaddr), a(op), a(val), a(timeout), a(uaddr2), a(val3))
+ -> (syscall(Sysfutex, a(uaddr), a(op), a(val), a(timeout), a(uaddr2), a(val3)) : int)
}
const semctl = {semid, semnum, cmd, arg
-> (syscall(Syssemctl, a(semnum), a(cmd), a(arg)) : int)
diff --git a/support/syscall-gen/types+freebsd-x64.frag b/support/syscall-gen/types+freebsd-x64.frag
index 9bae3054..f8990533 100644
--- a/support/syscall-gen/types+freebsd-x64.frag
+++ b/support/syscall-gen/types+freebsd-x64.frag
@@ -575,6 +575,13 @@ type uuid = struct
node : uint8[_Uuidnodesz];
;;
+const Umtxabstime = 1
+type umtx_time = struct
+ timeout : timespec
+ flags : uint32
+ clockid : uint32
+;;
+
/* open options */
const Ordonly : fdopt = 0x0
const Owronly : fdopt = 0x1
diff --git a/support/syscall-gen/types+linux-x64.frag b/support/syscall-gen/types+linux-x64.frag
index 13d95aaa..9e90a5ac 100644
--- a/support/syscall-gen/types+linux-x64.frag
+++ b/support/syscall-gen/types+linux-x64.frag
@@ -556,6 +556,8 @@ const Futexcmprequeuepi : futexop = 12
const Futexpriv : futexop = 128
const Futexclockrt : futexop = 256
+const Futexbitsetmatchany : int32 = -1
+
/* poll events : posix */
const Pollin : pollevt = 0x001 /* There is data to read. */
const Pollpri : pollevt = 0x002 /* There is urgent data to read. */
--
2.18.0
---- On Tue, 14 Aug 2018 23:15:39 -0700 Ori Bernstein <ori@xxxxxxxxxxxxxx> wrote ----
> Overall, seems sane, biggest issue is a missed wake that I let slip through earlier.
> Comments inline.
>
> On Sat, 11 Aug 2018 19:12:47 -0700, iriri <iri@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> >
> > diff --git a/lib/sys/sys+linux-x64.myr b/lib/sys/sys+linux-x64.myr
> > index 1cebb123..83fdea53 100644
> > --- a/lib/sys/sys+linux-x64.myr
> > +++ b/lib/sys/sys+linux-x64.myr
> > @@ -562,6 +562,8 @@ pkg sys =
> >
> > const Futexpriv : futexop = 128
> > const Futexclockrt : futexop = 256
> > +
> > + const Futexbitsetmatchany : int32 = -1
>
> Same note as previous patch.
>
> > /* poll events : posix */
> > const Pollin : pollevt = 0x001 /* There is data to read. */
> > @@ -1273,7 +1275,7 @@ pkg sys =
> > const dup : (fd : fd -> fd)
> > const dup2 : (src : fd, dst : fd -> fd)
> > const futex : (uaddr : int32#, op : futexop, val : int32, \
> > diff --git a/lib/thread/condvar+freebsd.myr b/lib/thread/condvar+freebsd.myr
> > index 002757ae..4e2b0c11 100644
> > --- a/lib/thread/condvar+freebsd.myr
> > +++ b/lib/thread/condvar+freebsd.myr
> > @@ -1,7 +1,6 @@
> > use std
> >
> > use "atomic"
> > -use "common"
> > use "mutex"
> > use "futex"
> >
> > @@ -22,19 +21,11 @@ const mkcond = {mtx
> > }
> >
> > const condwait = {cond
> > - var seq
> > - var mtx
> > + var mtx = cond._mtx
> > + var seq = xget(&cond._seq)
> >
> > - mtx = cond._mtx
> > - seq = cond._seq
> > mtxunlock(mtx)
> > -
> > - /*
> > - FIXME?: `ftxwait` can be interrupted but `condwait` should always be
> > - done in a loop anyway.
> > - */
> > - ftxwait(&cond._seq, seq, Zptr)
> > -
> > + ftxwait(&cond._seq, seq, 0)
> > mtxlock(mtx)
>
> Looking at this code, I realized it may have lost something
> in the translation from Linux cond vars. Isn't it possible
> that if we have multiple waiters on the condition variable,
> we could miss waking up a waiter, because we erronously decide
> a lock is uncontended?
>
> Consider this interleaving of operations:
>
> t1 t2 t3
> (st=U) | | |
> lock + | |
> condwait + | |
> (st=L) | | |
> unlock + | |
> (st=U) | | |
> | lock + |
> | (st=L) | |
> | condwait + |
> | (st=L) | |
> | unlock + |
> | (st=U) | |
> | | |
> | | signal +
> | | ftxwake |
> lock + | |
> (st=L) | | |
> dostuff | | |
> unlock + | |
> (st=U) | | |
>
> It seems to me that T2 will never get woken up unless
> we use the special mtxcontended() function in our futex
> based mutex implementation, which sets it to contended
> while locking the futexe. That forces the unlock to issue
> a wakeup, and wakes up the thread.
>
> This could probably use some more commenting. Possibly
> with the contents of the above message.
>
>
> > diff --git a/lib/thread/condvar+osx.myr b/lib/thread/condvar+osx.myr
> > index d74c321f..d18f4ccb 100644
> > --- a/lib/thread/condvar+osx.myr
> > +++ b/lib/thread/condvar+osx.myr
> > @@ -1,7 +1,6 @@
> > use std
> >
> > use "atomic"
> > -use "common"
> > use "mutex"
> > use "futex"
> >
> > @@ -22,19 +21,11 @@ const mkcond = {mtx
> > }
> >
> > const condwait = {cond
> > - var seq
> > - var mtx
> > + var mtx = cond._mtx
> > + var seq = xget(&cond._seq)
> >
> > - mtx = cond._mtx
> > - seq = cond._seq
> > mtxunlock(mtx)
> > -
> > - /*
> > - FIXME?: `ftxwait` can be interrupted but `condwait` should always be
> > - done in a loop anyway.
> > - */
> > - ftxwait(&cond._seq, seq, Zptr)
> > -
> > + ftxwait(&cond._seq, seq, 0)
> > mtxlock(mtx)
> > }
>
> Same bug, seems like we can have a missed wakeup. Also, it really looks like
> all of the condvar implementations could be unified, at least for the waiting.
> Is it the waking that keeps them separate right now?
>
> >
> > diff --git a/lib/thread/futex+freebsd.myr b/lib/thread/futex+freebsd.myr
> > index ea5f28d8..a9ad01ed 100644
> > --- a/lib/thread/futex+freebsd.myr
> > +++ b/lib/thread/futex+freebsd.myr
> > @@ -1,3 +1,4 @@
> > +use std
> > use sys
> >
> > use "atomic"
> > @@ -7,34 +8,51 @@ pkg thread =
> > type ftxtag = uint32
> > impl atomic ftxtag
> >
> > - const ftxwait : (uaddr : ftxtag#, val : ftxtag, timeout : sys.timespec# -> int)
> > - const ftxwake : (uaddr : ftxtag# -> int)
> > - const ftxwakeall : (uaddr : ftxtag# -> int)
> > + const ftxwait : (uaddr : ftxtag#, val : ftxtag, tmout : uint32 -> sys.errno)
> > + const ftxwake : (uaddr : ftxtag# -> void)
> > + const ftxwakeall : (uaddr : ftxtag# -> void)
> > ;;
> >
> > -const ftxwait = {uaddr, val, timeout
> > - if timeout == Zptr
> > - -> sys.umtx_op((uaddr : void#), sys.Umtxwaituintpriv, (val : uint64), Zptr, Zptr)
> > +const ftxwait = {uaddr, val, tmout
> > + var ut : sys._umtx_time, utp, utsize, rc
> > +
> > + if tmout == 0
>
> Preference: use -1 for infinite timeout.
>
> > + utp = Zptr
> > + utsize = Zptr
> > + else
> > + var t = (tmout : int64)
> > + std.assert(sys.clock_gettime(`sys.Clockmonotonic, &ut._timeout) == 0,
> > + "error: clock_gettime returned -1\n")
> > + ut._timeout.nsec += (t % 1_000_000) * 1000
> > + ut._timeout.sec += (ut._timeout.nsec / 1_000_000_000) + (t / 1_000_000)
> > + ut._timeout.nsec %= 1_000_000_000
> > + ut._flags = (sys.Umtxabstime : uint32)
> > + ut._clockid = 1 /* CLOCK_MONOTONIC. Not exported from sys. */
>
> Can always export from sys.
>
> > + utp = (&ut : void#)
> > + utsize = (sizeof(sys._umtx_time) : void#)
> > + ;;
> > +
> > + while (rc = (sys.umtx_op((uaddr : void#),
> > + sys.Umtxwaituintpriv,
> > + (val : uint64),
> > + utsize,
> > + utp) : sys.errno)) == sys.Eintr
> > ;;
> >
> > - var ut : sys._umtx_time = [
> > - ._timeout = timeout#,
> > - ._flags = (sys.Umtxabstime : uint32),
> > - ._clockid = 1, /* CLOCK_MONOTONIC. Not exported from sys. */
> > - ]
> > - -> sys.umtx_op((uaddr : void#),
> > - sys.Umtxwaituintpriv,
> > - (val : uint64),
> > - (sizeof(sys._umtx_time) : void#),
> > - (&ut : void#))
> > + match rc
> > + | 0: -> 0
> > + | sys.Eagain: -> sys.Eagain
> > + | sys.Etimedout: -> sys.Etimedout
> > + | err: std.fatal("error: futex returned {}\n", err)
>
> std.abort(), so we can get a nice core dump.
>
> > + ;;
> > }
> >
> > const ftxwake = {uaddr
> > - -> sys.umtx_op((uaddr : void#), sys.Umtxwakepriv, 1, Zptr, Zptr)
> > + sys.umtx_op((uaddr : void#), sys.Umtxwakepriv, 1, Zptr, Zptr)
> > }
> >
> > const ftxwakeall = {uaddr
> > - -> sys.umtx_op((uaddr : void#), sys.Umtxwakepriv, 0x7fffffff, Zptr, Zptr)
> > + sys.umtx_op((uaddr : void#), sys.Umtxwakepriv, 0x7fffffff, Zptr, Zptr)
> > }
> >
> > impl atomic ftxtag =
> > diff --git a/lib/thread/futex+linux.myr b/lib/thread/futex+linux.myr
> > index c5dbf062..d20aca8b 100644
> > --- a/lib/thread/futex+linux.myr
> > +++ b/lib/thread/futex+linux.myr
> > @@ -1,3 +1,4 @@
> > +use std
> > use sys
> >
> > use "atomic"
> > @@ -7,26 +8,58 @@ pkg thread =
> > type ftxtag = uint32
> > impl atomic ftxtag
> >
> > - const ftxwait : (uaddr : ftxtag#, val : ftxtag, timeout : sys.timespec# -> int)
> > - const ftxwake : (uaddr : ftxtag# -> int)
> > - const ftxwakeall : (uaddr : ftxtag# -> int)
> > + const ftxwait : (uaddr : ftxtag#, val : ftxtag, tmout : uint32 -> sys.errno)
> > + const ftxwake : (uaddr : ftxtag# -> void)
> > + const ftxwakeall : (uaddr : ftxtag# -> void)
> > ;;
> >
> > -const ftxwait = {uaddr, val, timeout
> > - -> (sys.futex((uaddr : int32#),
> > - sys.Futexwait | sys.Futexpriv,
> > - (val : int32),
> > - timeout,
> > - Zptr,
> > - 0) : int)
> > +const ftxwait = {uaddr, val, tmout
> > + var rc
> > +
>
> > + if tmout == 0
> > + while (rc = (sys.futex((uaddr : int32#),
> > + sys.Futexwait | sys.Futexpriv,
> > + (val : int32),
> > + Zptr,
> > + Zptr,
> > + 0) : sys.errno)) == sys.Eintr
> > + ;;
> > + else
> > + var t = (tmout : int64)
> > + var ts
> > + std.assert(sys.clock_gettime(`sys.Clockmonotonic, &ts) == 0,
> > + "error: clock_gettime returned -1\n")
> > + ts.nsec += (t % 1_000_000) * 1000
> > + ts.sec += (ts.nsec / 1_000_000_000) + (t / 1_000_000)
> > + ts.nsec %= 1_000_000_000
> > +
> > + /*
> > + Futexwaitbitset + Futexbitsetmatchany causes the timeout to be
> > + treated as absolute rather than relative.
> > + */
> > + while (rc = (sys.futex((uaddr : int32#),
> > + sys.Futexwaitbitset | sys.Futexpriv,
> > + (val : int32),
> > + &ts,
> > + Zptr,
> > + sys.Futexbitsetmatchany) : sys.errno)) == sys.Eintr
> > + ;;
> > + ;;
>
> This could be extracted into a utility function shared across all the
> implementations, now that the time struct is regularized. Something
> like:
>
> var tmptr = totimeout(&tm)
> while (rc = sys.futex(..., tmptr, ...) == sys.Eintr)
> break
> ;;
>
> > impl atomic ftxtag =
> > diff --git a/lib/thread/futex+openbsd:6.2.myr b/lib/thread/futex+openbsd:6.2.myr
> > index e3c9c413..cda80347 100644
> > --- a/lib/thread/futex+openbsd:6.2.myr
> > +++ b/lib/thread/futex+openbsd:6.2.myr
>
> Same comments as the other systems.
>
> > impl atomic ftxtag =
> > diff --git a/lib/thread/futex+osx.myr b/lib/thread/futex+osx.myr
> > index 17478703..c07e6b3e 100644
> > --- a/lib/thread/futex+osx.myr
> > +++ b/lib/thread/futex+osx.myr
> > }
> >
> Same comments as the other systems.
>
> > impl atomic ftxtag =
>
> --
> Ori Bernstein
>
>
| [PATCH 2/4] Fix futex timeouts and handle futex error codes. | iriri <iri@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> |
| Re: [PATCH 2/4] Fix futex timeouts and handle futex error codes. | Ori Bernstein <ori@xxxxxxxxxxxxxx> |
- Prev by Date: [PATCH 1/4] Make timespec/timeval struct members signed to simplify arithmetic.
- Next by Date: [PATCH 3/4] Add rwlocks.
- Previous by thread: Re: [PATCH 2/4] Fix futex timeouts and handle futex error codes.
- Next by thread: [PATCH 3/4] Add rwlocks.
- Index(es):