mem: Fix memory leak in handling of deferred snoops
authorAndreas Hansson <andreas.hansson@arm.com>
Thu, 26 May 2016 10:56:24 +0000 (11:56 +0100)
committerAndreas Hansson <andreas.hansson@arm.com>
Thu, 26 May 2016 10:56:24 +0000 (11:56 +0100)
This patch fixes a memory leak where deferred snoop packets never got
deallocated. On the call to MSHR::handleSnoop these snoops were
treated as if a response will be sent, as the MSHR was
pendingModified. Consequently, a copy of the packet was created and
added to the MSHR targets. However, an preceeding target to the same
MSHR, originally from a CPU, was serviced before the snoop, and caused
the block to be invalidated. This happens for ReadExReq and
UpgradeReq.

Note that the original snoop will receive a response, just not from
the cache in question, but instead from the cache upstream that issued
the ReadExReq or UpgradeReq.

Change-Id: I4ac012fbc8a46cf693ca390fe9476105d444e6f4
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
src/mem/cache/cache.cc

index 3048973fbc510775aa18eef749d87d3d4d5f14a4..e6247823c8488c5e1bf54bf6794db934b51f5889 100644 (file)
@@ -1954,6 +1954,19 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing,
     }
 
     if (!blk || !blk->isValid()) {
+        if (is_deferred) {
+            // we no longer have the block, and will not respond, but a
+            // packet was allocated in MSHR::handleSnoop and we have
+            // to delete it
+            assert(pkt->needsResponse());
+
+            // we have passed the block to a cache upstream, that
+            // cache should be responding
+            assert(pkt->cacheResponding());
+
+            delete pkt;
+        }
+
         DPRINTF(CacheVerbose, "%s snoop miss for %s addr %#llx size %d\n",
                 __func__, pkt->cmdString(), pkt->getAddr(), pkt->getSize());
         return snoop_delay;
@@ -2045,6 +2058,7 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing,
         // responding, then we've made a copy of both the request and
         // the packet, delete them here
         assert(pkt->needsResponse());
+        assert(!pkt->cacheResponding());
         delete pkt->req;
         delete pkt;
     }