From 721efa4d094197404a0df0d2bae6c27b4402c0ef Mon Sep 17 00:00:00 2001 From: Andreas Hansson Date: Fri, 12 Aug 2016 14:11:45 +0100 Subject: [PATCH] mem: Update mostly exclusive policy even further This patch takes yet another step in maintaining the clusivity, in that it allows a mostly-inclusive cache to hold on to blocks even when responding to a ReadExReq or UpgradeReq. Previously the cache simply invalidated these blocks, but there is no strict need to do so. The most important part of this patch is that we simply mark the block clean when satisfying the upstream request where the cache is allowed to keep the block. The only tricky part of the patch is in the memory management of deferred snoops, where we need to distinguish the cases where only the packet was copied (we expected to respond), and the cases where we created an entirely new packet and request (we kept it only to replay later). The code in satisfyRequest is definitely ready for some refactoring after this. Change-Id: I201ddc7b2582eaa46fb8cff0c7ad09e02d64b0fc Reviewed-by: Nikos Nikoleris Reviewed-by: Tony Gutierrez --- src/mem/cache/cache.cc | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc index 10f39db3d..e3f2777ef 100644 --- a/src/mem/cache/cache.cc +++ b/src/mem/cache/cache.cc @@ -200,16 +200,14 @@ Cache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, // sanity check assert(pkt->cmd == MemCmd::ReadExReq || pkt->cmd == MemCmd::SCUpgradeFailReq); + assert(!pkt->hasSharers()); // if we have a dirty copy, make sure the recipient // keeps it marked dirty (in the modified state) if (blk->isDirty()) { pkt->setCacheResponding(); + blk->status &= ~BlkDirty; } - // on ReadExReq we give up our copy unconditionally, - // even if this cache is mostly inclusive, we may want - // to revisit this - invalidateBlock(blk); } else if (blk->isWritable() && !pending_downgrade && !pkt->hasSharers() && pkt->cmd != MemCmd::ReadCleanReq) { @@ -260,12 +258,19 @@ Cache::satisfyRequest(PacketPtr pkt, CacheBlk *blk, pkt->setHasSharers(); } } - } else { - // Upgrade or Invalidate - assert(pkt->isUpgrade() || pkt->isInvalidate()); + } else if (pkt->isUpgrade()) { + // sanity check + assert(!pkt->hasSharers()); - // for invalidations we could be looking at the temp block - // (for upgrades we always allocate) + if (blk->isDirty()) { + // we were in the Owned state, and a cache above us that + // has the line in Shared state needs to be made aware + // that the data it already has is in fact dirty + pkt->setCacheResponding(); + blk->status &= ~BlkDirty; + } + } else { + assert(pkt->isInvalidate()); invalidateBlock(blk); DPRINTF(CacheVerbose, "%s for %s addr %#llx size %d (invalidation)\n", __func__, pkt->cmdString(), pkt->getAddr(), pkt->getSize()); @@ -956,7 +961,7 @@ Cache::createMissPacket(PacketPtr cpu_pkt, CacheBlk *blk, // if there are upstream caches that have already marked the // packet as having sharers (not passing writable), pass that info // downstream - if (cpu_pkt->hasSharers()) { + if (cpu_pkt->hasSharers() && !needsWritable) { // note that cpu_pkt may have spent a considerable time in the // MSHR queue and that the information could possibly be out // of date, however, there is no harm in conservatively @@ -2059,13 +2064,17 @@ Cache::handleSnoop(PacketPtr pkt, CacheBlk *blk, bool is_timing, } } - if (!respond && is_timing && is_deferred) { - // 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 + if (!respond && is_deferred) { assert(pkt->needsResponse()); - assert(!pkt->cacheResponding()); - delete pkt->req; + + // if we copied the deferred packet with the intention to + // respond, but are not responding, then a cache above us must + // be, and we can use this as the indication of whether this + // is a packet where we created a copy of the request or not + if (!pkt->cacheResponding()) { + delete pkt->req; + } + delete pkt; }