Matthew Poremba has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/39175 )
Change subject: mem-ruby: Fix race related to atomics in VIPER
......................................................................
mem-ruby: Fix race related to atomics in VIPER
There is a race condition in VIPER where an atomic issued to the same
address can occur resulting in multiple trigger messages signalling the
compleition of the atomic operation. The first message was deallocating
the TBE causing the second message to dereference a nullptr when looking
up the TBE.
A counter is added to track the number of in flight AtomicDone trigger
messages. The AtomicDone is not called until the last in flight message
arrives at the trigger queue. The remaining messages call AtomicNotDone
which simply pops the message from the queue and keeps the TBE
allocated.
Change-Id: Ie1de0436861a7c393ad6d2fb2faceb83c18d4cc3
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39175
Reviewed-by: Matt Sinclair <[email protected]>
Reviewed-by: Jason Lowe-Power <[email protected]>
Maintainer: Matt Sinclair <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/mem/ruby/protocol/GPU_VIPER-TCC.sm
1 file changed, 12 insertions(+), 1 deletion(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Matt Sinclair: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/mem/ruby/protocol/GPU_VIPER-TCC.sm
b/src/mem/ruby/protocol/GPU_VIPER-TCC.sm
index 5edd7db..e21ba99 100644
--- a/src/mem/ruby/protocol/GPU_VIPER-TCC.sm
+++ b/src/mem/ruby/protocol/GPU_VIPER-TCC.sm
@@ -108,6 +108,7 @@
MachineID From, desc="Waiting for writeback from...";
NetDest Destination, desc="Data destination";
int numAtomics, desc="number remaining atomics";
+ int atomicDoneCnt, desc="number AtomicDones triggered";
}
structure(TBETable, external="yes") {
@@ -256,9 +257,17 @@
peek(triggerQueue_in, TriggerMsg) {
TBE tbe := TBEs.lookup(in_msg.addr);
Entry cache_entry := getCacheEntry(in_msg.addr);
- if (tbe.numAtomics == 0) {
+
+ // There is a possible race where multiple AtomicDone triggers can
be
+ // sent if another Atomic to the same address is issued after the
+ // AtomicDone is triggered but before the message arrives here. For
+ // that case we count the number of AtomicDones in flight for this
+ // address and only call AtomicDone to deallocate the TBE when it
is
+ // the last in flight message.
+ if (tbe.numAtomics == 0 && tbe.atomicDoneCnt == 1) {
trigger(Event:AtomicDone, in_msg.addr, cache_entry, tbe);
} else {
+ tbe.atomicDoneCnt := tbe.atomicDoneCnt - 1;
trigger(Event:AtomicNotDone, in_msg.addr, cache_entry, tbe);
}
}
@@ -453,6 +462,7 @@
set_tbe(TBEs.lookup(address));
tbe.Destination.clear();
tbe.numAtomics := 0;
+ tbe.atomicDoneCnt := 0;
}
if (coreRequestNetwork_in.isReady(clockEdge())) {
peek(coreRequestNetwork_in, CPURequestMsg) {
@@ -573,6 +583,7 @@
tbe.numAtomics := tbe.numAtomics - 1;
if (tbe.numAtomics==0) {
enqueue(triggerQueue_out, TriggerMsg, 1) {
+ tbe.atomicDoneCnt := tbe.atomicDoneCnt + 1;
out_msg.addr := address;
out_msg.Type := TriggerType:AtomicDone;
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/39175
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: Ie1de0436861a7c393ad6d2fb2faceb83c18d4cc3
Gerrit-Change-Number: 39175
Gerrit-PatchSet: 3
Gerrit-Owner: Matthew Poremba <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Jason Lowe-Power <[email protected]>
Gerrit-Reviewer: Kyle Roarty <[email protected]>
Gerrit-Reviewer: Matt Sinclair <[email protected]>
Gerrit-Reviewer: Matthew Poremba <[email protected]>
Gerrit-Reviewer: kokoro <[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
