cpu, sim-se: don't wake up threads that are awake in futex
authorCiro Santilli <ciro.santilli@arm.com>
Tue, 8 Oct 2019 17:23:15 +0000 (18:23 +0100)
committerCiro Santilli <ciro.santilli@arm.com>
Thu, 24 Oct 2019 12:59:42 +0000 (12:59 +0000)
FutexMap::wakeup is called when the futex(TGT_FUTEX_WAKE syscall is done.

FutexMap maintains a list of sleeping threads for each futex address
added on FutexMap::suspend, and entries are removed from the list
at FutexMap::wakeup.

The problem is that this system was not taking into account that threads
can be woken up by memory accesses to locked addresses via the path:

SimpleThread::activate
BaseSimpleCPU::wakeup
AbstractMemory::checkLockedAddrList
AbstractMemory::access
DRAMCtrl::recvAtomic
CoherentXBar::recvAtomicBackdoor
SimpleExecContext::writeMem

which happens on trivial pthread examples on ARM at least. The instruction
that locked memory in those test cases was LDAXR.

This could lead futex(TGT_FUTEX_WAKE to awake a thread that is already
awake but is first on the sleeping thread list, instead of a sleeping one,
which can lead all threads to incorrectly sleep and in turn to
"simulate() limit reached".

To implement this, ThreadContext::activate return now returns a boolean
that indicates if the state changed. suspend and halt are also modified
to also return a boolean in the same case for symmetry, although this is
not strictly necessary for the current patch.

Change-Id: Ia6b4d3e6148c64721d810b8f1fffaa208a394b06
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/21606
Reviewed-by: Brandon Potter <Brandon.Potter@amd.com>
Maintainer: Brandon Potter <Brandon.Potter@amd.com>
Tested-by: kokoro <noreply+kokoro@google.com>
src/sim/futex_map.hh

index 3d34109be744ed8e8693e42b9dcae2ba1b9ff544..5e60f7c376cb51e5f1329fe431054d94023a5dfd 100644 (file)
@@ -153,9 +153,16 @@ class FutexMap : public std::unordered_map<FutexKey, WaiterList>
         auto &waiterList = it->second;
 
         while (!waiterList.empty() && woken_up < count) {
-            waiterList.front().tc->activate();
+            // Threads may be woken up by access to locked
+            // memory addresses outside of syscalls, so we
+            // must only count threads that were actually
+            // woken up by this syscall.
+            auto& tc = waiterList.front().tc;
+            if (tc->status() != ThreadContext::Active) {
+                tc->activate();
+                woken_up++;
+            }
             waiterList.pop_front();
-            woken_up++;
         }
 
         if (waiterList.empty())