From: Ciro Santilli <ciro.santilli@arm.com>
Date: Tue, 8 Oct 2019 17:23:15 +0000 (+0100)
Subject: cpu, sim-se: don't wake up threads that are awake in futex
X-Git-Tag: v19.0.0.0~413
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=f60383d060ed82400526a47a1ba3182253e9f2f5;p=gem5.git

cpu, sim-se: don't wake up threads that are awake in futex

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>
---

diff --git a/src/sim/futex_map.hh b/src/sim/futex_map.hh
index 3d34109be..5e60f7c37 100644
--- a/src/sim/futex_map.hh
+++ b/src/sim/futex_map.hh
@@ -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())