From e3e808416f92d5558d17731fd31c002d84d40181 Mon Sep 17 00:00:00 2001 From: Andreas Hansson Date: Thu, 26 May 2016 11:56:24 +0100 Subject: [PATCH] mem: Fix memory leak in handling of deferred snoops 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 --- src/mem/cache/cache.cc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc index 3048973fb..e6247823c 100644 --- a/src/mem/cache/cache.cc +++ b/src/mem/cache/cache.cc @@ -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; }