From f60383d060ed82400526a47a1ba3182253e9f2f5 Mon Sep 17 00:00:00 2001 From: Ciro Santilli Date: Tue, 8 Oct 2019 18:23:15 +0100 Subject: [PATCH] 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 Maintainer: Brandon Potter Tested-by: kokoro --- src/sim/futex_map.hh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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 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()) -- 2.30.2