ruby: fix deadlock bug in banked array resource checks
authorDavid Hashe <david.hashe@amd.com>
Mon, 20 Jul 2015 14:15:18 +0000 (09:15 -0500)
committerDavid Hashe <david.hashe@amd.com>
Mon, 20 Jul 2015 14:15:18 +0000 (09:15 -0500)
The Ruby banked array resource checks (initiated from SLICC) did a check and
allocate at the same time. If a transition needs more than one resource, then
it might check/allocate resource #1, then fail to get resource #2. Another
transition might then try to get the same resources, but in reverse order.
Deadlock.

This patch separates resource checking and resource reservation into two
steps to avoid deadlock.

src/mem/protocol/RubySlicc_Types.sm
src/mem/ruby/structures/BankedArray.cc
src/mem/ruby/structures/BankedArray.hh
src/mem/ruby/structures/CacheMemory.cc
src/mem/ruby/structures/CacheMemory.hh

index 6c3c4168d181486df373fe9f18e55fc8de2a9155..11159fd6c09bc938b47c5b92b4397b39a94713b2 100644 (file)
@@ -154,7 +154,7 @@ structure (CacheMemory, external = "yes") {
   Cycles getTagLatency();
   Cycles getDataLatency();
   void setMRU(Address);
-  void recordRequestType(CacheRequestType);
+  void recordRequestType(CacheRequestType, Address);
   bool checkResourceAvailable(CacheResourceType, Address);
 
   int getCacheSize();
index dbde2ab9af9680e9941e1b878418e24973c8650b..8bc3cf584cc4bea0c5af9f59caa85b3108622a90 100644 (file)
@@ -58,13 +58,29 @@ BankedArray::tryAccess(int64 idx)
     assert(bank < banks);
 
     if (busyBanks[bank].endAccess >= curTick()) {
-        if (!(busyBanks[bank].startAccess == curTick() &&
-            busyBanks[bank].idx == idx)) {
             return false;
+    }
+
+    return true;
+}
+
+void
+BankedArray::reserve(int64 idx)
+{
+    if (accessLatency == 0)
+        return;
+
+    unsigned int bank = mapIndexToBank(idx);
+    assert(bank < banks);
+
+    if(busyBanks[bank].endAccess >= curTick()) {
+        if (busyBanks[bank].startAccess == curTick() &&
+             busyBanks[bank].idx == idx) {
+            // this is the same reservation (can happen when
+            // e.g., reserve the same resource for read and write)
+            return; // OK
         } else {
-            // We tried to allocate resources twice
-            // in the same cycle for the same addr
-            return true;
+            panic("BankedArray reservation error");
         }
     }
 
@@ -72,8 +88,6 @@ BankedArray::tryAccess(int64 idx)
     busyBanks[bank].startAccess = curTick();
     busyBanks[bank].endAccess = curTick() +
         (accessLatency-1) * m_ruby_system->clockPeriod();
-
-    return true;
 }
 
 unsigned int
index dbfee9994064de3a2664f4c5efce64ff31481361..438186944a2dde59af529e1162e13017c051c530 100644 (file)
@@ -70,6 +70,8 @@ class BankedArray
     // This is so we don't get aliasing on blocks being replaced
     bool tryAccess(int64 idx);
 
+    void reserve(int64 idx);
+
     Cycles getLatency() const { return accessLatency; }
 };
 
index c802d8fb6ad0e0a5f15b77d8bf9a843a7b87df21..d08724cff17caaf4efa557ca73aff78a398ba800 100644 (file)
@@ -528,22 +528,32 @@ CacheMemory::regStats()
         ;
 }
 
+// assumption: SLICC generated files will only call this function
+// once **all** resources are granted
 void
-CacheMemory::recordRequestType(CacheRequestType requestType)
+CacheMemory::recordRequestType(CacheRequestType requestType, Address addr)
 {
     DPRINTF(RubyStats, "Recorded statistic: %s\n",
             CacheRequestType_to_string(requestType));
     switch(requestType) {
     case CacheRequestType_DataArrayRead:
+        if (m_resource_stalls)
+            dataArray.reserve(addressToCacheSet(addr));
         numDataArrayReads++;
         return;
     case CacheRequestType_DataArrayWrite:
+        if (m_resource_stalls)
+            dataArray.reserve(addressToCacheSet(addr));
         numDataArrayWrites++;
         return;
     case CacheRequestType_TagArrayRead:
+        if (m_resource_stalls)
+            tagArray.reserve(addressToCacheSet(addr));
         numTagArrayReads++;
         return;
     case CacheRequestType_TagArrayWrite:
+        if (m_resource_stalls)
+            tagArray.reserve(addressToCacheSet(addr));
         numTagArrayWrites++;
         return;
     default:
index c9c20d8b8c6588fcfdb73a81a58eb64c59ccb56d..af5e680d8d65079a8b48390115e3dc332f475648 100644 (file)
@@ -117,7 +117,7 @@ class CacheMemory : public SimObject
 
     void regStats();
     bool checkResourceAvailable(CacheResourceType res, Address addr);
-    void recordRequestType(CacheRequestType requestType);
+    void recordRequestType(CacheRequestType requestType, Address addr);
 
   public:
     Stats::Scalar m_demand_hits;