diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2021-04-09 11:53:58 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2021-04-13 17:41:39 +0200 |
commit | a7fabe2328d840bc6f1f7a11c2435057b63c94d9 (patch) | |
tree | e110186f320df40311dcb33f6b01b4559e3fe239 /src | |
parent | 385e242ba987bfd611daa21ea981ad4963822b8c (diff) |
Light cleanup in QSemaphore Futex implementation
Gets rid of a goto, fixes overflow detection with wakeAll set,
and fixes 64-bit futex mode with futexHasWaiterCount = false.
Change-Id: I8bb98118013fc1dc2a8a405845bec0cb3350494f
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/corelib/thread/qsemaphore.cpp | 62 |
1 files changed, 27 insertions, 35 deletions
diff --git a/src/corelib/thread/qsemaphore.cpp b/src/corelib/thread/qsemaphore.cpp index 213852250f..d89367c1cb 100644 --- a/src/corelib/thread/qsemaphore.cpp +++ b/src/corelib/thread/qsemaphore.cpp @@ -132,8 +132,8 @@ static constexpr bool futexHasWaiterCount = true; static constexpr bool futexHasWaiterCount = false; #endif -static const quintptr futexNeedsWakeAllBit = - Q_UINT64_C(1) << (sizeof(quintptr) * CHAR_BIT - 1); +static constexpr quintptr futexNeedsWakeAllBit = futexHasWaiterCount ? + (Q_UINT64_C(1) << (sizeof(quintptr) * CHAR_BIT - 1)) : 0x80000000U; static int futexAvailCounter(quintptr v) { @@ -169,6 +169,7 @@ static QBasicAtomicInteger<quint32> *futexLow32(QBasicAtomicInteger<quintptr> *p static QBasicAtomicInteger<quint32> *futexHigh32(QBasicAtomicInteger<quintptr> *ptr) { + Q_ASSERT(futexHasWaiterCount); auto result = reinterpret_cast<QBasicAtomicInteger<quint32> *>(ptr); #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN && QT_POINTER_SIZE > 4 ++result; @@ -184,38 +185,16 @@ futexSemaphoreTryAcquire_loop(QBasicAtomicInteger<quintptr> &u, quintptr curValu int n = int(unsigned(nn)); // we're called after one testAndSet, so start by waiting first - goto start_wait; - - forever { - if (futexAvailCounter(curValue) >= n) { - // try to acquire - quintptr newValue = curValue - nn; - if (u.testAndSetOrdered(curValue, newValue, curValue)) - return true; // succeeded! - continue; - } - - // not enough tokens available, put us to wait - if (remainingTime == 0) - return false; - + for (;;) { // indicate we're waiting -start_wait: - auto ptr = [&u]() { - if constexpr (futexHasWaiterCount) - return futexLow32(&u); - else - return &u; - }(); + auto ptr = futexLow32(&u); if (n > 1 || !futexHasWaiterCount) { u.fetchAndOrRelaxed(futexNeedsWakeAllBit); curValue |= futexNeedsWakeAllBit; if constexpr (futexHasWaiterCount) { - if (n > 1) { - ptr = futexHigh32(&u); - // curValue >>= 32; // but this is UB in 32-bit, so roundabout: - curValue = quint64(curValue) >> 32; - } + Q_ASSERT(n > 1); + ptr = futexHigh32(&u); + curValue >>= 32; } } @@ -230,6 +209,17 @@ start_wait: curValue = u.loadAcquire(); if (IsTimed) remainingTime = timer.remainingTimeNSecs(); + + // try to acquire + while (futexAvailCounter(curValue) >= n) { + quintptr newValue = curValue - nn; + if (u.testAndSetOrdered(curValue, newValue, curValue)) + return true; // succeeded! + } + + // not enough tokens available, put us to wait + if (remainingTime == 0) + return false; } } @@ -252,12 +242,14 @@ template <bool IsTimed> bool futexSemaphoreTryAcquire(QBasicAtomicInteger<quintp return false; // we need to wait - quintptr oneWaiter = quintptr(Q_UINT64_C(1) << 32); // zero on 32-bit - if (futexHasWaiterCount) { + constexpr quintptr oneWaiter = quintptr(Q_UINT64_C(1) << 32); // zero on 32-bit + if constexpr (futexHasWaiterCount) { // We don't use the fetched value from above so futexWait() fails if // it changed after the testAndSetOrdered above. - if ((quint64(curValue) >> 32) == 0x7fffffff) - return false; // overflow! + if (((curValue >> 32) & 0x7fffffffU) == 0x7fffffffU) { + qCritical() << "Waiter count overflow in QSemaphore"; + return false; + } // increase the waiter count u.fetchAndAddRelaxed(oneWaiter); @@ -270,6 +262,8 @@ template <bool IsTimed> bool futexSemaphoreTryAcquire(QBasicAtomicInteger<quintp if (futexSemaphoreTryAcquire_loop<IsTimed>(u, curValue, nn, timeout)) return true; + Q_ASSERT(IsTimed); + if (futexHasWaiterCount) { // decrement the number of threads waiting Q_ASSERT(futexHigh32(&u)->loadRelaxed() & 0x7fffffffU); @@ -501,8 +495,6 @@ bool QSemaphore::tryAcquire(int n, int timeout) return false; d->avail -= n; return true; - - } /*! |