Kyle Roarty has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/31996 )

Change subject: mem-ruby: resolve race between data and DMA in MOESI_AMD_Base-dir
......................................................................

mem-ruby: resolve race between data and DMA in MOESI_AMD_Base-dir

There seems to be race condition while running several benchmarks, where
the DMA engine and the CorePair simultaneously send requests for the
same block. This patch fixes two scenarios
(a) If the request from the DMA engine arrives before the one from the
CorePair, the directory controller records it as a pending request.
However, once the DMA request is serviced, the directory doesn't check
for pending requests. The CorePair, consequently, never sees a response
to its request and this results in a Deadlock.

Added call to wakeUpDependents in the transition from BDR_Pm to U
Added call to wakeUpDependents in the transition from BDW_P to U

(b) If the request from the CorePair is being serviced by the directory
and the DMA requests for the same block, this causes an invalid
transition because the current coherence doesn't take care of this
scenario.

Added transition state where the requests from DMA are added to the
stall buffer.

Updated B to U CoreUnblock transition to check all buffers, as the DMA
requests were being placed later in the stall buffer than was being checked

Change-Id: I5a76efef97723bc53cf239ea7e112f84fc874ef8
---
M src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
M src/mem/ruby/slicc_interface/AbstractController.cc
2 files changed, 22 insertions(+), 3 deletions(-)



diff --git a/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm b/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
index c8dafd5..f1bc637 100644
--- a/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
+++ b/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
@@ -180,6 +180,7 @@
   void set_tbe(TBE a);
   void unset_tbe();
   void wakeUpAllBuffers();
+  void wakeUpAllBuffers(Addr a);
   void wakeUpBuffers(Addr a);
   Cycles curCycle();

@@ -1069,6 +1070,10 @@
     stall_and_wait(requestNetwork_in, address);
   }

+ action(sd_stallAndWaitRequest, "sd", desc="Stall and wait on the address") {
+    stall_and_wait(dmaRequestQueue_in, address);
+  }
+
action(wa_wakeUpDependents, "wa", desc="Wake up any requests waiting for this address") {
     wakeUpBuffers(address);
   }
@@ -1077,6 +1082,10 @@
     wakeUpAllBuffers();
   }

+ action(wa_wakeUpAllDependentsAddr, "waaa", desc="Wake up any requests waiting for this address") {
+    wakeUpAllBuffers(address);
+  }
+
   action(z_stall, "z", desc="...") {
   }

@@ -1090,6 +1099,11 @@
       st_stallAndWaitRequest;
   }

+ // The exit state is always going to be U, so wakeUpDependents logic should be covered in all the
+  // transitions which are flowing into U.
+ transition({BL, BS_M, BM_M, B_M, BP, BDW_P, BS_PM, BM_PM, B_PM, BS_Pm, BM_Pm, B_Pm, B}, {DmaRead,DmaWrite}){
+    sd_stallAndWaitRequest;
+  }

   // transitions from U
   transition(U, DmaRead, BDR_PM) {L3TagArrayRead} {
@@ -1193,7 +1207,7 @@
   }

   transition({B}, CoreUnblock, U) {
-    wa_wakeUpDependents;
+    wa_wakeUpAllDependentsAddr;
     pu_popUnblockQueue;
   }

@@ -1323,12 +1337,18 @@
   }

   transition(BDW_P, ProbeAcksComplete, U) {
+ // Check for pending requests from the core we put to sleep while waiting
+    // for a response
+    wa_wakeUpAllDependentsAddr;
     dt_deallocateTBE;
     pt_popTriggerQueue;
   }

   transition(BDR_Pm, ProbeAcksComplete, U) {
     dd_sendResponseDmaData;
+ // Check for pending requests from the core we put to sleep while waiting
+    // for a response
+    wa_wakeUpDependents;
     dt_deallocateTBE;
     pt_popTriggerQueue;
   }
diff --git a/src/mem/ruby/slicc_interface/AbstractController.cc b/src/mem/ruby/slicc_interface/AbstractController.cc
index 9da8727..d2b3370 100644
--- a/src/mem/ruby/slicc_interface/AbstractController.cc
+++ b/src/mem/ruby/slicc_interface/AbstractController.cc
@@ -149,8 +149,7 @@
 {
     if (m_waiting_buffers.count(addr) > 0) {
         //
- // Wake up all possible lower rank (i.e. lower priority) buffers that could
-        // be waiting on this message.
+ // Wake up all possible buffers that could be waiting on this message.
         //
         for (int in_port_rank = m_in_ports - 1;
              in_port_rank >= 0;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/31996
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I5a76efef97723bc53cf239ea7e112f84fc874ef8
Gerrit-Change-Number: 31996
Gerrit-PatchSet: 1
Gerrit-Owner: Kyle Roarty <kyleroarty1...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to