mem: Do not use sender state to track forwarded snoops in cache

This patch changes how the cache tracks which snoops are forwarded,
and which ones are created locally. Previously the identification was
based on an empty sender state of a specific class, but this method
fails to distinguish which cache actually attached the sender
state. Instead we use the same mechanism as the crossbar, and keep
track of the requests that have outstanding snoops.

diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc
index 187ed48019cf90dff3aae342f597f42a01f9369c..fb37f524b309874e9e89a446df4bb523fd9e5a87 100644 (file)
--- a/src/mem/cache/cache.cc
+++ b/src/mem/cache/cache.cc
@@ -467,14 +467,6 @@ Cache::access(PacketPtr pkt, CacheBlk *&blk, Cycles &lat,
     return false;
 }
 
-
-class ForwardResponseRecord : public Packet::SenderState
-{
-  public:
-
-    ForwardResponseRecord() {}
-};
-
 void
 Cache::doWritebacks(PacketList& writebacks, Tick forward_time)
 {
@@ -556,27 +548,27 @@ Cache::recvTimingSnoopResp(PacketPtr pkt)
             pkt->cmdString(), pkt->getAddr(), pkt->getSize());
 
     assert(pkt->isResponse());
-
-    // must be cache-to-cache response from upper to lower level
-    ForwardResponseRecord *rec =
-        dynamic_cast<ForwardResponseRecord *>(pkt->senderState);
     assert(!system->bypassCaches());
 
-    if (rec == NULL) {
-        // @todo What guarantee do we have that this HardPFResp is
-        // actually for this cache, and not a cache closer to the
-        // memory?
+    // determine if the response is from a snoop request we created
+    // (in which case it should be in the outstandingSnoop), or if we
+    // merely forwarded someone else's snoop request
+    const bool forwardAsSnoop = outstandingSnoop.find(pkt->req) ==
+        outstandingSnoop.end();
+
+    if (!forwardAsSnoop) {
+        // the packet came from this cache, so sink it here and do not
+        // forward it
         assert(pkt->cmd == MemCmd::HardPFResp);
-        // Check if it's a prefetch response and handle it. We shouldn't
-        // get any other kinds of responses without FRRs.
-        DPRINTF(Cache, "Got prefetch response from above for addr %#llx (%s)\n",
-                pkt->getAddr(), pkt->isSecure() ? "s" : "ns");
+
+        outstandingSnoop.erase(pkt->req);
+
+        DPRINTF(Cache, "Got prefetch response from above for addr "
+                "%#llx (%s)\n", pkt->getAddr(), pkt->isSecure() ? "s" : "ns");
         recvTimingResp(pkt);
         return;
     }
 
-    pkt->popSenderState();
-    delete rec;
     // forwardLatency is set here because there is a response from an
     // upper level cache.
     // To pay the delay that occurs if the packet comes from the bus,
@@ -1897,7 +1889,6 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing,
             // there is a snoop hit in upper levels
             Packet snoopPkt(pkt, true, true);
             snoopPkt.setExpressSnoop();
-            snoopPkt.pushSenderState(new ForwardResponseRecord());
             // the snoop packet does not need to wait any additional
             // time
             snoopPkt.headerDelay = snoopPkt.payloadDelay = 0;
@@ -1912,10 +1903,6 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing,
                 // cache-to-cache response from some upper cache
                 assert(!alreadyResponded);
                 pkt->assertMemInhibit();
-            } else {
-                // no cache (or anyone else for that matter) will
-                // respond, so delete the ForwardResponseRecord here
-                delete snoopPkt.popSenderState();
             }
             if (snoopPkt.sharedAsserted()) {
                 pkt->assertShared();
@@ -2350,6 +2337,8 @@ Cache::getTimingPacket()
         // may result in the MSHR being prematurely deallocated.
 
         if (snoop_pkt.memInhibitAsserted()) {
+            auto M5_VAR_USED r = outstandingSnoop.insert(snoop_pkt.req);
+            assert(r.second);
             // If we are getting a non-shared response it is dirty
             bool pending_dirty_resp = !snoop_pkt.sharedAsserted();
             markInService(mshr, pending_dirty_resp);

diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index 5ea72831be0b26d9921179ce59461a59860bb69c..4f1763c8938e66eb7f5f0c60ed13e5b69dfde76e 100644 (file)
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -246,6 +246,13 @@ class Cache : public BaseCache
     EventWrapper<Cache, &Cache::writebackTempBlockAtomic> \
         writebackTempBlockAtomicEvent;
 
+    /**
+     * Store the outstanding requests that we are expecting snoop
+     * responses from so we can determine which snoop responses we
+     * generated and which ones were merely forwarded.
+     */
+    std::unordered_set<RequestPtr> outstandingSnoop;
+
     /**
      * Does all the processing necessary to perform the provided request.
      * @param pkt The memory request to perform.