mem: Fix cache sender state handling and add clarification

[gem5.git] / src / mem / cache / cache.cc

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 29919ccdfe263c2e328700c8f4bae95cdcf0fa04..187ed48019cf90dff3aae342f597f42a01f9369c 100644 (file)
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -2005,15 +2005,9 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing,
     }
 
     if (!respond && is_timing && is_deferred) {
-        // if it's a deferred timing snoop then we've made a copy of
-        // both the request and the packet, and so if we're not using
-        // those copies to respond and delete them here
-        DPRINTF(Cache, "Deleting pkt %p and request %p for cmd %s addr: %p\n",
-                pkt, pkt->req, pkt->cmdString(), pkt->getAddr());
-
-        // the packets needs a response (just not from us), so we also
-        // need to delete the request and not rely on the packet
-        // destructor
+        // if it's a deferred timing snoop to which we are not
+        // responding, then we've made a copy of both the request and
+        // the packet, delete them here
         assert(pkt->needsResponse());
         delete pkt->req;
         delete pkt;
@@ -2336,6 +2330,10 @@ Cache::getTimingPacket()
         // dirty one.
         Packet snoop_pkt(tgt_pkt, true, false);
         snoop_pkt.setExpressSnoop();
+        // We are sending this packet upwards, but if it hits we will
+        // get a snoop response that we end up treating just like a
+        // normal response, hence it needs the MSHR as its sender
+        // state
         snoop_pkt.senderState = mshr;
         cpuSidePort->sendTimingSnoopReq(&snoop_pkt);
 
@@ -2405,7 +2403,9 @@ Cache::getTimingPacket()
     }
 
     assert(pkt != NULL);
-    pkt->senderState = mshr;
+    // play it safe and append (rather than set) the sender state, as
+    // forwarded packets may already have existing state
+    pkt->pushSenderState(mshr);
     return pkt;
 }