AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Light cleanup in QSemaphore Futex implementation

Browse Source 
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>
This commit is contained in:
Allan Sandfeld Jensen 2021-04-09 11:53:58 +02:00
parent 385e242ba9
commit a7fabe2328
1 changed files with 27 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
src/corelib/thread/qsemaphore.cpp
View File

@ -132,8 +132,8 @@ static constexpr bool futexHasWaiterCount = true;
static constexpr bool futexHasWaiterCount = false; static constexpr bool futexHasWaiterCount = false;
#endif #endif
static const quintptr futexNeedsWakeAllBit = static constexpr quintptr futexNeedsWakeAllBit = futexHasWaiterCount ?
Q_UINT64_C(1) << (sizeof(quintptr) * CHAR_BIT - 1); (Q_UINT64_C(1) << (sizeof(quintptr) * CHAR_BIT - 1)) : 0x80000000U;
static int futexAvailCounter(quintptr v) static int futexAvailCounter(quintptr v)
{ {
@ -169,6 +169,7 @@ static QBasicAtomicInteger<quint32> *futexLow32(QBasicAtomicInteger<quintptr> *p
static QBasicAtomicInteger<quint32> *futexHigh32(QBasicAtomicInteger<quintptr> *ptr) static QBasicAtomicInteger<quint32> *futexHigh32(QBasicAtomicInteger<quintptr> *ptr)
{ {
Q_ASSERT(futexHasWaiterCount);
auto result = reinterpret_cast<QBasicAtomicInteger<quint32> *>(ptr); auto result = reinterpret_cast<QBasicAtomicInteger<quint32> *>(ptr);
#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN && QT_POINTER_SIZE > 4 #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN && QT_POINTER_SIZE > 4
++result; ++result;
@ -184,38 +185,16 @@ futexSemaphoreTryAcquire_loop(QBasicAtomicInteger<quintptr> &u, quintptr curValu
int n = int(unsigned(nn)); int n = int(unsigned(nn));
// we're called after one testAndSet, so start by waiting first // we're called after one testAndSet, so start by waiting first
goto start_wait; for (;;) {
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;
// indicate we're waiting // indicate we're waiting
start_wait: auto ptr = futexLow32(&u);
auto ptr = [&u]() {
if constexpr (futexHasWaiterCount)
return futexLow32(&u);
else
return &u;
}();
if (n > 1 || !futexHasWaiterCount) { if (n > 1 || !futexHasWaiterCount) {
u.fetchAndOrRelaxed(futexNeedsWakeAllBit); u.fetchAndOrRelaxed(futexNeedsWakeAllBit);
curValue |= futexNeedsWakeAllBit; curValue |= futexNeedsWakeAllBit;
if constexpr (futexHasWaiterCount) { if constexpr (futexHasWaiterCount) {
if (n > 1) { Q_ASSERT(n > 1);
ptr = futexHigh32(&u); ptr = futexHigh32(&u);
// curValue >>= 32; // but this is UB in 32-bit, so roundabout: curValue >>= 32;
curValue = quint64(curValue) >> 32;
}
} }
} }
@ -230,6 +209,17 @@ start_wait:
curValue = u.loadAcquire(); curValue = u.loadAcquire();
if (IsTimed) if (IsTimed)
remainingTime = timer.remainingTimeNSecs(); 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; return false;
// we need to wait // we need to wait
quintptr oneWaiter = quintptr(Q_UINT64_C(1) << 32); // zero on 32-bit constexpr quintptr oneWaiter = quintptr(Q_UINT64_C(1) << 32); // zero on 32-bit
if (futexHasWaiterCount) { if constexpr (futexHasWaiterCount) {
// We don't use the fetched value from above so futexWait() fails if // We don't use the fetched value from above so futexWait() fails if
// it changed after the testAndSetOrdered above. // it changed after the testAndSetOrdered above.
if ((quint64(curValue) >> 32) == 0x7fffffff) if (((curValue >> 32) & 0x7fffffffU) == 0x7fffffffU) {
return false; // overflow! qCritical() << "Waiter count overflow in QSemaphore";
return false;
}
// increase the waiter count // increase the waiter count
u.fetchAndAddRelaxed(oneWaiter); u.fetchAndAddRelaxed(oneWaiter);
@ -270,6 +262,8 @@ template <bool IsTimed> bool futexSemaphoreTryAcquire(QBasicAtomicInteger<quintp
if (futexSemaphoreTryAcquire_loop<IsTimed>(u, curValue, nn, timeout)) if (futexSemaphoreTryAcquire_loop<IsTimed>(u, curValue, nn, timeout))
return true; return true;
Q_ASSERT(IsTimed);
if (futexHasWaiterCount) { if (futexHasWaiterCount) {
// decrement the number of threads waiting // decrement the number of threads waiting
Q_ASSERT(futexHigh32(&u)->loadRelaxed() & 0x7fffffffU); Q_ASSERT(futexHigh32(&u)->loadRelaxed() & 0x7fffffffU);
@ -501,8 +495,6 @@ bool QSemaphore::tryAcquire(int n, int timeout)
return false; return false;
d->avail -= n; d->avail -= n;
return true; return true;
} }
/*! /*!