Kyle Roarty has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/31996 )
Change subject: mem-ruby: fix races between data and DMA in
MOESI_AMD_Base-dir
......................................................................
mem-ruby: fix races between data and DMA in MOESI_AMD_Base-dir
There are race conditions 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
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/31996
Reviewed-by: Matt Sinclair <[email protected]>
Reviewed-by: Bradford Beckmann <[email protected]>
Maintainer: Bradford Beckmann <[email protected]>
Tested-by: kokoro <[email protected]>
---
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(-)
Approvals:
Bradford Beckmann: Looks good to me, approved; Looks good to me, approved
Matt Sinclair: Looks good to me, but someone else must approve
kokoro: Regressions pass
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: 3
Gerrit-Owner: Kyle Roarty <[email protected]>
Gerrit-Reviewer: Bradford Beckmann <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Kyle Roarty <[email protected]>
Gerrit-Reviewer: Matt Sinclair <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-CC: Alexandru Duțu <[email protected]>
Gerrit-CC: Anthony Gutierrez <[email protected]>
Gerrit-CC: GAURAV JAIN <[email protected]>
Gerrit-CC: John Alsop <[email protected]>
Gerrit-CC: Matthew Poremba <[email protected]>
Gerrit-CC: Pouya Fotouhi <[email protected]>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
