From 1b20df5607e86d3b384716792274fe01fa4f3f80 Mon Sep 17 00:00:00 2001 From: Steve Reinhardt Date: Tue, 26 Jun 2007 22:23:10 -0700 Subject: [PATCH] Handle deferred snoops better. --HG-- extra : convert_revision : 703da6128832eb0d5cfed7724e5105f4b3fe4f90 --- src/mem/cache/cache.hh | 6 ++- src/mem/cache/cache_impl.hh | 34 ++++++++------- src/mem/cache/miss/mshr.cc | 82 +++++++++++++++++++++++-------------- src/mem/cache/miss/mshr.hh | 3 +- src/mem/cache/tags/lru.cc | 5 ++- src/mem/tport.cc | 13 +++++- 6 files changed, 91 insertions(+), 52 deletions(-) diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh index 2a95dc53c..161fb801d 100644 --- a/src/mem/cache/cache.hh +++ b/src/mem/cache/cache.hh @@ -185,14 +185,16 @@ class Cache : public BaseCache void satisfyCpuSideRequest(PacketPtr pkt, BlkType *blk); bool satisfyMSHR(MSHR *mshr, PacketPtr pkt, BlkType *blk); - void doTimingSupplyResponse(PacketPtr req_pkt, uint8_t *blk_data); + void doTimingSupplyResponse(PacketPtr req_pkt, uint8_t *blk_data, + bool already_copied); /** * Sets the blk to the new state. * @param blk The cache block being snooped. * @param new_state The new coherence state for the block. */ - void handleSnoop(PacketPtr ptk, BlkType *blk, bool is_timing); + void handleSnoop(PacketPtr ptk, BlkType *blk, + bool is_timing, bool is_deferred); /** * Create a writeback request for the given block. diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index a73612f24..599eecc82 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -622,7 +622,7 @@ Cache::satisfyMSHR(MSHR *mshr, PacketPtr pkt, } else { // response to snoop request DPRINTF(Cache, "processing deferred snoop...\n"); - handleSnoop(target->pkt, blk, true); + handleSnoop(target->pkt, blk, true, true); } mshr->popTarget(); @@ -678,12 +678,10 @@ Cache::handleResponse(PacketPtr pkt) pkt->getAddr()); BlkType *blk = tags->findBlock(pkt->getAddr()); - if (blk == NULL && pkt->cmd == MemCmd::UpgradeResp) { - if (!mshr->handleReplacedPendingUpgrade(pkt)) { - mq->markPending(mshr); - requestMemSideBus((RequestCause)mq->index, pkt->finishTime); - return; - } + if (!mshr->handleFill(pkt, blk)) { + mq->markPending(mshr); + requestMemSideBus((RequestCause)mq->index, pkt->finishTime); + return; } PacketList writebacks; @@ -814,10 +812,12 @@ Cache::handleFill(PacketPtr pkt, BlkType *blk, template void Cache::doTimingSupplyResponse(PacketPtr req_pkt, - uint8_t *blk_data) + uint8_t *blk_data, + bool already_copied) { - // timing-mode snoop responses require a new packet - PacketPtr pkt = new Packet(req_pkt); + // timing-mode snoop responses require a new packet, unless we + // already made a copy... + PacketPtr pkt = already_copied ? req_pkt : new Packet(req_pkt); pkt->allocate(); pkt->makeTimingResponse(); pkt->setDataFromBlock(blk_data, blkSize); @@ -827,7 +827,7 @@ Cache::doTimingSupplyResponse(PacketPtr req_pkt, template void Cache::handleSnoop(PacketPtr pkt, BlkType *blk, - bool is_timing) + bool is_timing, bool is_deferred) { if (!blk || !blk->isValid()) { return; @@ -854,9 +854,10 @@ Cache::handleSnoop(PacketPtr pkt, BlkType *blk, } if (supply) { + assert(!pkt->memInhibitAsserted()); pkt->assertMemInhibit(); if (is_timing) { - doTimingSupplyResponse(pkt, blk->data); + doTimingSupplyResponse(pkt, blk->data, is_deferred); } else { pkt->makeAtomicResponse(); pkt->setDataFromBlock(blk->data, blkSize); @@ -892,6 +893,8 @@ Cache::snoopTiming(PacketPtr pkt) // better not be snooping a request that conflicts with something // we have outstanding... if (mshr && mshr->inService) { + DPRINTF(Cache, "Deferring snoop on in-service MSHR to blk %x\n", + blk_addr); mshr->allocateSnoopTarget(pkt, curTick, order++); if (mshr->getNumTargets() > numTarget) warn("allocating bonus target for snoop"); //handle later @@ -913,6 +916,7 @@ Cache::snoopTiming(PacketPtr pkt) assert(wb_pkt->cmd == MemCmd::Writeback); if (pkt->isRead()) { + assert(!pkt->memInhibitAsserted()); pkt->assertMemInhibit(); if (!pkt->needsExclusive()) { pkt->assertShared(); @@ -922,7 +926,7 @@ Cache::snoopTiming(PacketPtr pkt) // the packet's invalidate flag is set... assert(pkt->isInvalidate()); } - doTimingSupplyResponse(pkt, wb_pkt->getPtr()); + doTimingSupplyResponse(pkt, wb_pkt->getPtr(), false); } if (pkt->isInvalidate()) { @@ -933,7 +937,7 @@ Cache::snoopTiming(PacketPtr pkt) } } - handleSnoop(pkt, blk, true); + handleSnoop(pkt, blk, true, false); } @@ -948,7 +952,7 @@ Cache::snoopAtomic(PacketPtr pkt) } BlkType *blk = tags->findBlock(pkt->getAddr()); - handleSnoop(pkt, blk, false); + handleSnoop(pkt, blk, false, false); return hitLatency; } diff --git a/src/mem/cache/miss/mshr.cc b/src/mem/cache/miss/mshr.cc index ca5e38601..23645cb27 100644 --- a/src/mem/cache/miss/mshr.cc +++ b/src/mem/cache/miss/mshr.cc @@ -75,6 +75,7 @@ MSHR::allocate(Addr _addr, int _size, PacketPtr target, assert(deferredTargets.empty()); deferredNeedsExclusive = false; pendingInvalidate = false; + pendingShared = false; replacedPendingUpgrade = false; data = NULL; } @@ -120,7 +121,7 @@ MSHR::allocateTarget(PacketPtr target, Tick when, Counter _order) } void -MSHR::allocateSnoopTarget(PacketPtr target, Tick when, Counter _order) +MSHR::allocateSnoopTarget(PacketPtr pkt, Tick when, Counter _order) { assert(inService); // don't bother to call otherwise @@ -130,23 +131,33 @@ MSHR::allocateSnoopTarget(PacketPtr target, Tick when, Counter _order) return; } - if (needsExclusive) { - // We're awaiting an exclusive copy, so ownership is pending. - // It's up to us to respond once the data arrives. - target->assertMemInhibit(); - } + DPRINTF(Cache, "deferred snoop on %x: %s %s\n", addr, + needsExclusive ? "needsExclusive" : "", + pkt->needsExclusive() ? "pkt->needsExclusive()" : ""); - if (target->needsExclusive()) { - // This transaction will take away our pending copy - pendingInvalidate = true; + if (needsExclusive || pkt->needsExclusive()) { + // actual target device (typ. PhysicalMemory) will delete the + // packet on reception, so we need to save a copy here + targets.push_back(Target(new Packet(pkt), when, _order, false)); + ++ntargets; + + if (needsExclusive) { + // We're awaiting an exclusive copy, so ownership is pending. + // It's up to us to respond once the data arrives. + pkt->assertMemInhibit(); + } + + if (pkt->needsExclusive()) { + // This transaction will take away our pending copy + pendingInvalidate = true; + } } else { - // We'll keep our pending copy, but we can't let the other guy - // think he's getting it exclusive - target->assertShared(); + // Read to a read: no conflict, so no need to record as + // target, but make sure neither reader thinks he's getting an + // exclusive copy + pendingShared = true; + pkt->assertShared(); } - - targets.push_back(Target(target, when, _order, false)); - ++ntargets; } @@ -164,6 +175,7 @@ MSHR::promoteDeferredTargets() needsExclusive = deferredNeedsExclusive; pendingInvalidate = false; + pendingShared = false; deferredNeedsExclusive = false; order = targets.front().order; readyTick = std::max(curTick, targets.front().time); @@ -200,25 +212,33 @@ MSHR::handleReplacement(CacheBlk *blk, int blkSize) bool -MSHR::handleReplacedPendingUpgrade(Packet *pkt) +MSHR::handleFill(Packet *pkt, CacheBlk *blk) { - // @TODO: if upgrade is nacked and replacedPendingUpgradeDirty is true, then we need to writeback the data (or rel - assert(pkt->cmd == MemCmd::UpgradeResp); - assert(replacedPendingUpgrade); - replacedPendingUpgrade = false; // reset - if (replacedPendingUpgradeDirty) { - // we wrote back the previous copy; just reissue as a ReadEx - return false; + if (replacedPendingUpgrade) { + // block was replaced while upgrade request was in service + assert(pkt->cmd == MemCmd::UpgradeResp); + assert(blk == NULL); + assert(replacedPendingUpgrade); + replacedPendingUpgrade = false; // reset + if (replacedPendingUpgradeDirty) { + // we wrote back the previous copy; just reissue as a ReadEx + return false; + } + + // previous copy was not dirty, but we are now owner... fake out + // cache by taking saved data and converting UpgradeResp to + // ReadExResp + assert(data); + pkt->cmd = MemCmd::ReadExResp; + pkt->setData(data); + delete [] data; + data = NULL; + } else if (pendingShared) { + // we snooped another read while this read was in + // service... assert shared line on its behalf + pkt->assertShared(); } - // previous copy was not dirty, but we are now owner... fake out - // cache by taking saved data and converting UpgradeResp to - // ReadExResp - assert(data); - pkt->cmd = MemCmd::ReadExResp; - pkt->setData(data); - delete [] data; - data = NULL; return true; } diff --git a/src/mem/cache/miss/mshr.hh b/src/mem/cache/miss/mshr.hh index a9380d99a..07fe5c96c 100644 --- a/src/mem/cache/miss/mshr.hh +++ b/src/mem/cache/miss/mshr.hh @@ -105,6 +105,7 @@ class MSHR : public Packet::SenderState bool deferredNeedsExclusive; bool pendingInvalidate; + bool pendingShared; /** Is there a pending upgrade that got replaced? */ bool replacedPendingUpgrade; bool replacedPendingUpgradeDirty; @@ -213,7 +214,7 @@ public: bool promoteDeferredTargets(); void handleReplacement(CacheBlk *blk, int blkSize); - bool handleReplacedPendingUpgrade(Packet *pkt); + bool handleFill(Packet *pkt, CacheBlk *blk); /** * Prints the contents of this MSHR to stderr. diff --git a/src/mem/cache/tags/lru.cc b/src/mem/cache/tags/lru.cc index fa46aff7b..3269aa4db 100644 --- a/src/mem/cache/tags/lru.cc +++ b/src/mem/cache/tags/lru.cc @@ -207,6 +207,9 @@ LRU::findReplacement(Addr addr, PacketList &writebacks) totalRefs += blk->refCount; ++sampledRefs; blk->refCount = 0; + + DPRINTF(Cache, "set %x: selecting blk %x for replacement\n", + set, regenerateBlkAddr(blk->tag, set)); } else if (!blk->isTouched) { tagsInUse++; blk->isTouched = true; @@ -216,8 +219,6 @@ LRU::findReplacement(Addr addr, PacketList &writebacks) } } - DPRINTF(Cache, "set %x: selecting blk %x for replacement\n", - set, regenerateBlkAddr(blk->tag, set)); return blk; } diff --git a/src/mem/tport.cc b/src/mem/tport.cc index 0a2127490..6c8c12ce2 100644 --- a/src/mem/tport.cc +++ b/src/mem/tport.cc @@ -69,11 +69,21 @@ SimpleTimingPort::recvTiming(PacketPtr pkt) // if we ever added it back. assert(pkt->isRequest()); assert(pkt->result == Packet::Unknown); + + if (pkt->memInhibitAsserted()) { + // snooper will supply based on copy of packet + // still target's responsibility to delete packet + delete pkt->req; + delete pkt; + return true; + } + bool needsResponse = pkt->needsResponse(); Tick latency = recvAtomic(pkt); // turn packet around to go back to requester if response expected if (needsResponse) { - // recvAtomic() should already have turned packet into atomic response + // recvAtomic() should already have turned packet into + // atomic response assert(pkt->isResponse()); pkt->convertAtomicToTimingResponse(); schedSendTiming(pkt, curTick + latency); @@ -81,6 +91,7 @@ SimpleTimingPort::recvTiming(PacketPtr pkt) delete pkt->req; delete pkt; } + return true; }