Reviewed-by: Marek Olšák <marek.ol...@amd.com>
Marek
On Mon, Nov 6, 2017 at 11:23 AM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> From: Nicolai Hähnle <nicolai.haeh...@amd.com>
>
> v2:
> - style fixes
> - fix missing timeout handling in futex path
>
> Reviewed-by: Marek Olšák <marek.ol...@amd.com> (v1)
> ---
> src/util/futex.h | 9 ++++--
> src/util/simple_mtx.h | 2 +-
> src/util/u_queue.c | 82
> ++++++++++++++++++++++++++++++++++++++++++++++++++-
> src/util/u_queue.h | 54 ++++++++++++++++++++-------------
> 4 files changed, 121 insertions(+), 26 deletions(-)
>
> diff --git a/src/util/futex.h b/src/util/futex.h
> index fa42cf4cf59..330e18f03a2 100644
> --- a/src/util/futex.h
> +++ b/src/util/futex.h
> @@ -26,33 +26,36 @@
>
> #if defined(HAVE_FUTEX)
>
> #include <limits.h>
> #include <stdint.h>
> #include <unistd.h>
> #include <linux/futex.h>
> #include <sys/syscall.h>
> #include <sys/time.h>
>
> -static inline long sys_futex(void *addr1, int op, int val1, struct timespec
> *timeout, void *addr2, int val3)
> +static inline long sys_futex(void *addr1, int op, int val1, const struct
> timespec *timeout, void *addr2, int val3)
> {
> return syscall(SYS_futex, addr1, op, val1, timeout, addr2, val3);
> }
>
> static inline int futex_wake(uint32_t *addr)
> {
> return sys_futex(addr, FUTEX_WAKE, 1, NULL, NULL, 0);
> }
>
> static inline int futex_wake_all(uint32_t *addr)
> {
> return sys_futex(addr, FUTEX_WAKE, INT_MAX, NULL, NULL, 0);
> }
>
> -static inline int futex_wait(uint32_t *addr, int32_t value)
> +static inline int futex_wait(uint32_t *addr, int32_t value, const struct
> timespec *timeout)
> {
> - return sys_futex(addr, FUTEX_WAIT, value, NULL, NULL, 0);
> + /* FUTEX_WAIT_BITSET with FUTEX_BITSET_MATCH_ANY is equivalent to
> + * FUTEX_WAIT, except that it treats the timeout as absolute. */
> + return sys_futex(addr, FUTEX_WAIT_BITSET, value, timeout, NULL,
> + FUTEX_BITSET_MATCH_ANY);
> }
>
> #endif
>
> #endif /* UTIL_FUTEX_H */
> diff --git a/src/util/simple_mtx.h b/src/util/simple_mtx.h
> index 0c2602d03b6..4186a2d4d64 100644
> --- a/src/util/simple_mtx.h
> +++ b/src/util/simple_mtx.h
> @@ -72,21 +72,21 @@ simple_mtx_destroy(simple_mtx_t *mtx)
> static inline void
> simple_mtx_lock(simple_mtx_t *mtx)
> {
> uint32_t c;
>
> c = __sync_val_compare_and_swap(&mtx->val, 0, 1);
> if (__builtin_expect(c != 0, 0)) {
> if (c != 2)
> c = __sync_lock_test_and_set(&mtx->val, 2);
> while (c != 0) {
> - futex_wait(&mtx->val, 2);
> + futex_wait(&mtx->val, 2, NULL);
> c = __sync_lock_test_and_set(&mtx->val, 2);
> }
> }
> }
>
> static inline void
> simple_mtx_unlock(simple_mtx_t *mtx)
> {
> uint32_t c;
>
> diff --git a/src/util/u_queue.c b/src/util/u_queue.c
> index 706ee8b04d9..43c28ac6ef8 100644
> --- a/src/util/u_queue.c
> +++ b/src/util/u_queue.c
> @@ -19,20 +19,23 @@
> * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> * USE OR OTHER DEALINGS IN THE SOFTWARE.
> *
> * The above copyright notice and this permission notice (including the
> * next paragraph) shall be included in all copies or substantial portions
> * of the Software.
> */
>
> #include "u_queue.h"
>
> +#include <time.h>
> +
> +#include "util/os_time.h"
> #include "util/u_string.h"
> #include "util/u_thread.h"
>
> static void util_queue_killall_and_wait(struct util_queue *queue);
>
> /****************************************************************************
> * Wait for all queues to assert idle when exit() is called.
> *
> * Otherwise, C++ static variable destructors can be called while threads
> * are using the static variables.
> @@ -84,39 +87,116 @@ remove_from_atexit_list(struct util_queue *queue)
> break;
> }
> }
> mtx_unlock(&exit_mutex);
> }
>
> /****************************************************************************
> * util_queue_fence
> */
>
> +#ifdef UTIL_QUEUE_FENCE_FUTEX
> +static bool
> +do_futex_fence_wait(struct util_queue_fence *fence,
> + bool timeout, int64_t abs_timeout)
> +{
> + uint32_t v = fence->val;
> + struct timespec ts;
> + ts.tv_sec = abs_timeout / (1000*1000*1000);
> + ts.tv_nsec = abs_timeout % (1000*1000*1000);
> +
> + while (v != 0) {
> + if (v != 2) {
> + v = p_atomic_cmpxchg(&fence->val, 1, 2);
> + if (v == 0)
> + return true;
> + }
> +
> + int r = futex_wait(&fence->val, 2, timeout ? &ts : NULL);
> + if (timeout && r < 0) {
> + if (errno == -ETIMEDOUT)
> + return false;
> + }
> +
> + v = fence->val;
> + }
> +
> + return true;
> +}
> +
> +void
> +_util_queue_fence_wait(struct util_queue_fence *fence)
> +{
> + do_futex_fence_wait(fence, false, 0);
> +}
> +
> +bool
> +_util_queue_fence_wait_timeout(struct util_queue_fence *fence,
> + int64_t abs_timeout)
> +{
> + return do_futex_fence_wait(fence, true, abs_timeout);
> +}
> +
> +#endif
> +
> #ifdef UTIL_QUEUE_FENCE_STANDARD
> void
> util_queue_fence_signal(struct util_queue_fence *fence)
> {
> mtx_lock(&fence->mutex);
> fence->signalled = true;
> cnd_broadcast(&fence->cond);
> mtx_unlock(&fence->mutex);
> }
>
> void
> -util_queue_fence_wait(struct util_queue_fence *fence)
> +_util_queue_fence_wait(struct util_queue_fence *fence)
> {
> mtx_lock(&fence->mutex);
> while (!fence->signalled)
> cnd_wait(&fence->cond, &fence->mutex);
> mtx_unlock(&fence->mutex);
> }
>
> +bool
> +_util_queue_fence_wait_timeout(struct util_queue_fence *fence,
> + int64_t abs_timeout)
> +{
> + /* This terrible hack is made necessary by the fact that we really want an
> + * internal interface consistent with os_time_*, but cnd_timedwait is
> spec'd
> + * to be relative to the TIME_UTC clock.
> + */
> + int64_t rel = abs_timeout - os_time_get_nano();
> +
> + if (rel > 0) {
> + struct timespec ts;
> +
> + timespec_get(&ts, TIME_UTC);
> +
> + ts.tv_sec += abs_timeout / (1000*1000*1000);
> + ts.tv_nsec += abs_timeout % (1000*1000*1000);
> + if (ts.tv_nsec >= (1000*1000*1000)) {
> + ts.tv_sec++;
> + ts.tv_nsec -= (1000*1000*1000);
> + }
> +
> + mtx_lock(&fence->mutex);
> + while (!fence->signalled) {
> + if (cnd_timedwait(&fence->cond, &fence->mutex, &ts) != thrd_success)
> + break;
> + }
> + mtx_unlock(&fence->mutex);
> + }
> +
> + return fence->signalled;
> +}
> +
> void
> util_queue_fence_init(struct util_queue_fence *fence)
> {
> memset(fence, 0, sizeof(*fence));
> (void) mtx_init(&fence->mutex, mtx_plain);
> cnd_init(&fence->cond);
> fence->signalled = true;
> }
>
> void
> diff --git a/src/util/u_queue.h b/src/util/u_queue.h
> index 9c911fe6b4d..7f9b43d72ff 100644
> --- a/src/util/u_queue.h
> +++ b/src/util/u_queue.h
> @@ -74,40 +74,20 @@ util_queue_fence_init(struct util_queue_fence *fence)
> }
>
> static inline void
> util_queue_fence_destroy(struct util_queue_fence *fence)
> {
> assert(fence->val == 0);
> /* no-op */
> }
>
> static inline void
> -util_queue_fence_wait(struct util_queue_fence *fence)
> -{
> - uint32_t v = fence->val;
> -
> - if (likely(v == 0))
> - return;
> -
> - do {
> - if (v != 2) {
> - v = p_atomic_cmpxchg(&fence->val, 1, 2);
> - if (v == 0)
> - return;
> - }
> -
> - futex_wait(&fence->val, 2);
> - v = fence->val;
> - } while(v != 0);
> -}
> -
> -static inline void
> util_queue_fence_signal(struct util_queue_fence *fence)
> {
> uint32_t val = p_atomic_xchg(&fence->val, 0);
>
> assert(val != 0);
>
> if (val == 2)
> futex_wake_all(&fence->val);
> }
>
> @@ -140,21 +120,20 @@ util_queue_fence_is_signalled(struct util_queue_fence
> *fence)
> * Put this into your job structure.
> */
> struct util_queue_fence {
> mtx_t mutex;
> cnd_t cond;
> int signalled;
> };
>
> void util_queue_fence_init(struct util_queue_fence *fence);
> void util_queue_fence_destroy(struct util_queue_fence *fence);
> -void util_queue_fence_wait(struct util_queue_fence *fence);
> void util_queue_fence_signal(struct util_queue_fence *fence);
>
> /**
> * Move \p fence back into unsignalled state.
> *
> * \warning The caller must ensure that no other thread may currently be
> * waiting (or about to wait) on the fence.
> */
> static inline void
> util_queue_fence_reset(struct util_queue_fence *fence)
> @@ -163,20 +142,53 @@ util_queue_fence_reset(struct util_queue_fence *fence)
> fence->signalled = 0;
> }
>
> static inline bool
> util_queue_fence_is_signalled(struct util_queue_fence *fence)
> {
> return fence->signalled != 0;
> }
> #endif
>
> +void
> +_util_queue_fence_wait(struct util_queue_fence *fence);
> +
> +static inline void
> +util_queue_fence_wait(struct util_queue_fence *fence)
> +{
> + if (unlikely(!util_queue_fence_is_signalled(fence)))
> + _util_queue_fence_wait(fence);
> +}
> +
> +bool
> +_util_queue_fence_wait_timeout(struct util_queue_fence *fence,
> + int64_t abs_timeout);
> +
> +/**
> + * Wait for the fence to be signaled with a timeout.
> + *
> + * \param fence the fence
> + * \param abs_timeout the absolute timeout in nanoseconds, relative to the
> + * clock provided by os_time_get_nano.
> + *
> + * \return true if the fence was signaled, false if the timeout occurred.
> + */
> +static inline bool
> +util_queue_fence_wait_timeout(struct util_queue_fence *fence,
> + int64_t abs_timeout)
> +{
> + if (util_queue_fence_is_signalled(fence))
> + return true;
> +
> + return _util_queue_fence_wait_timeout(fence, abs_timeout);
> +}
> +
> typedef void (*util_queue_execute_func)(void *job, int thread_index);
>
> struct util_queue_job {
> void *job;
> struct util_queue_fence *fence;
> util_queue_execute_func execute;
> util_queue_execute_func cleanup;
> };
>
> /* Put this into your context. */
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
