Matthew Poremba has uploaded this change for review. (
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
---
M src/mem/ruby/protocol/GPU_VIPER-TCC.sm
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/mem/ruby/protocol/GPU_VIPER-TCC.sm
b/src/mem/ruby/protocol/GPU_VIPER-TCC.sm
index 5edd7db..12097ae 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) {
+
+ // The is a possible race where multiple AtomicDone triggers can be
+ // sent if another Atomic to the same address is issues 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: 1
Gerrit-Owner: Matthew Poremba <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
