From: Gabe Black <gabeblack@google.com>
Date: Mon, 22 Apr 2019 22:24:40 +0000 (-0700)
Subject: systemc: Add a distinct async_request_update mechanism.
X-Git-Tag: v19.0.0.0~918
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=ca3dd5087800a3400f60589ce2063d30ce602580;p=gem5.git

systemc: Add a distinct async_request_update mechanism.

This mechanism had just been plumbed into the regular request_update,
but that doesn't have any thread safety which is the whole point of
async_request_update. This new mechanism puts async update requests
into their own list which is checked any time normal updates happen.

The delta cycle which triggers those updates must happen through some
other means which will usually be ok. The exact timing of the update
is undefined, so it would be legal for it to either not be recognized
before the impending end of the simulation, or for it to get picked up
by subsequent activity. If there isn't subsequent activity but the
simulation also doesn't end, for instance if there are only gem5 events
left, then that update could be lost. That is an unresolved issue.

It would be nice to schedule a "ready" event if async updates were
added which would ensure they wouldn't starve. Unfortunately that
requires the event queue lock, and in practice it's been found that a
systemc process might block, effectively holding the event queue lock,
while it waits for some asyncrhonous update to give it something to do.
This effectively deadlocks the system since the update is blocked on
the lock the main thread holds, and the main thread is blocked waiting
for the update.

Change-Id: I580303db01673faafc2e63545b6a69b3327a521c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/18288
Reviewed-by: Gabe Black <gabeblack@google.com>
Maintainer: Gabe Black <gabeblack@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
---

diff --git a/src/systemc/core/channel.cc b/src/systemc/core/channel.cc
index 04f158bea..4c731716d 100644
--- a/src/systemc/core/channel.cc
+++ b/src/systemc/core/channel.cc
@@ -54,8 +54,7 @@ Channel::requestUpdate()
 void
 Channel::asyncRequestUpdate()
 {
-    //TODO This should probably not request an update directly.
-    scheduler.requestUpdate(this);
+    scheduler.asyncRequestUpdate(this);
 }
 
 std::set<Channel *> allChannels;
diff --git a/src/systemc/core/scheduler.cc b/src/systemc/core/scheduler.cc
index d06ddfb58..f1a78198d 100644
--- a/src/systemc/core/scheduler.cc
+++ b/src/systemc/core/scheduler.cc
@@ -257,6 +257,13 @@ Scheduler::requestUpdate(Channel *c)
         scheduleReadyEvent();
 }
 
+void
+Scheduler::asyncRequestUpdate(Channel *c)
+{
+    std::lock_guard<std::mutex> lock(asyncListMutex);
+    asyncUpdateList.pushLast(c);
+}
+
 void
 Scheduler::scheduleReadyEvent()
 {
@@ -321,6 +328,12 @@ void
 Scheduler::runUpdate()
 {
     status(StatusUpdate);
+    {
+        std::lock_guard<std::mutex> lock(asyncListMutex);
+        Channel *channel;
+        while ((channel = asyncUpdateList.getNext()) != nullptr)
+            updateList.pushLast(channel);
+    }
 
     try {
         Channel *channel = updateList.getNext();
diff --git a/src/systemc/core/scheduler.hh b/src/systemc/core/scheduler.hh
index 63f6ac35d..b576bec00 100644
--- a/src/systemc/core/scheduler.hh
+++ b/src/systemc/core/scheduler.hh
@@ -32,6 +32,7 @@
 
 #include <functional>
 #include <map>
+#include <mutex>
 #include <set>
 #include <vector>
 
@@ -191,6 +192,8 @@ class Scheduler
 
     // Schedule an update for a given channel.
     void requestUpdate(Channel *c);
+    // Same as above, but may be called from a different thread.
+    void asyncRequestUpdate(Channel *c);
 
     // Run the given process immediately, preempting whatever may be running.
     void
@@ -481,6 +484,9 @@ class Scheduler
 
     ChannelList updateList;
 
+    ChannelList asyncUpdateList;
+    std::mutex asyncListMutex;
+
     std::map<::Event *, Tick> eventsToSchedule;
 
     std::set<TraceFile *> traceFiles;