From 995146ead7bcf03b80bdea6281fa4a225ad48b72 Mon Sep 17 00:00:00 2001 From: Ron Dreslinski Date: Tue, 10 Oct 2006 17:10:56 -0400 Subject: [PATCH] Fix some more mem leaks, still some left Update retry mechanism src/mem/cache/base_cache.cc: Rework the retry mechanism src/mem/cache/base_cache.hh: Rework the retry mechanism Try to fix memory bug src/mem/cache/cache_impl.hh: Rework upgrades to not be blocked by slave src/mem/cache/miss/mshr_queue.cc: Fix mem leak on writebacks --HG-- extra : convert_revision : 3cec234ee441edf398ec8d0f51a0c5d7ada1e2be --- src/mem/cache/base_cache.cc | 57 ++++++++++++++++++-------------- src/mem/cache/base_cache.hh | 12 ++++++- src/mem/cache/cache_impl.hh | 48 +++++++++++---------------- src/mem/cache/miss/mshr_queue.cc | 5 +++ 4 files changed, 68 insertions(+), 54 deletions(-) diff --git a/src/mem/cache/base_cache.cc b/src/mem/cache/base_cache.cc index 5f9906dbc..0141fa2a0 100644 --- a/src/mem/cache/base_cache.cc +++ b/src/mem/cache/base_cache.cc @@ -45,6 +45,7 @@ BaseCache::CachePort::CachePort(const std::string &_name, BaseCache *_cache, { blocked = false; cshrRetry = NULL; + waitingOnRetry = false; //Start ports at null if more than one is created we should panic //cpuSidePort = NULL; //memSidePort = NULL; @@ -113,25 +114,30 @@ void BaseCache::CachePort::recvRetry() { Packet *pkt; + assert(waitingOnRetry); if (!drainList.empty()) { //We have some responses to drain first - bool result = true; - while (result && !drainList.empty()) { - result = sendTiming(drainList.front()); - if (result) - drainList.pop_front(); + if (sendTiming(drainList.front())) { + drainList.pop_front(); + if (!drainList.empty() || + !isCpuSide && cache->doMasterRequest() || + isCpuSide && cache->doSlaveRequest()) { + BaseCache::CacheEvent * reqCpu = new BaseCache::CacheEvent(this); + reqCpu->schedule(curTick + 1); + } + waitingOnRetry = false; } - if (!result) return; } else if (!isCpuSide) { - if (!cache->doMasterRequest()) return; + assert(cache->doMasterRequest()); pkt = cache->getPacket(); MSHR* mshr = (MSHR*)pkt->senderState; bool success = sendTiming(pkt); DPRINTF(Cache, "Address %x was %s in sending the timing request\n", pkt->getAddr(), success ? "succesful" : "unsuccesful"); cache->sendResult(pkt, mshr, success); + waitingOnRetry = !success; if (success && cache->doMasterRequest()) { //Still more to issue, rerequest in 1 cycle @@ -140,12 +146,14 @@ BaseCache::CachePort::recvRetry() reqCpu->schedule(curTick + 1); } } - else if (cshrRetry) + else { + assert(cshrRetry); //pkt = cache->getCoherencePacket(); //We save the packet, no reordering on CSHRS pkt = cshrRetry; bool success = sendTiming(pkt); + waitingOnRetry = !success; if (success && cache->doSlaveRequest()) { //Still more to issue, rerequest in 1 cycle @@ -154,7 +162,6 @@ BaseCache::CachePort::recvRetry() reqCpu->schedule(curTick + 1); cshrRetry = NULL; } - } return; } @@ -198,23 +205,22 @@ BaseCache::CacheEvent::CacheEvent(CachePort *_cachePort, Packet *_pkt) void BaseCache::CacheEvent::process() { - if (!cachePort->drainList.empty()) { - //We have some responses to drain first - bool result = true; - while (result && !cachePort->drainList.empty()) { - result = cachePort->sendTiming(cachePort->drainList.front()); - if (result) - cachePort->drainList.pop_front(); - } - if (!result) return; - } - if (!pkt) { - if (!cachePort->isCpuSide) + if (cachePort->waitingOnRetry) return; + //We have some responses to drain first + if (!cachePort->drainList.empty()) { + if (cachePort->sendTiming(cachePort->drainList.front())) { + cachePort->drainList.pop_front(); + if (!cachePort->drainList.empty() || + !cachePort->isCpuSide && cachePort->cache->doMasterRequest() || + cachePort->isCpuSide && cachePort->cache->doSlaveRequest()) + this->schedule(curTick + 1); + } + else cachePort->waitingOnRetry = true; + } + else if (!cachePort->isCpuSide) { - //For now, doMasterRequest somehow is still getting set - if (!cachePort->cache->doMasterRequest()) return; //MSHR pkt = cachePort->cache->getPacket(); MSHR* mshr = (MSHR*) pkt->senderState; @@ -222,6 +228,7 @@ BaseCache::CacheEvent::process() DPRINTF(Cache, "Address %x was %s in sending the timing request\n", pkt->getAddr(), success ? "succesful" : "unsuccesful"); cachePort->cache->sendResult(pkt, mshr, success); + cachePort->waitingOnRetry = !success; if (success && cachePort->cache->doMasterRequest()) { //Still more to issue, rerequest in 1 cycle @@ -237,6 +244,7 @@ BaseCache::CacheEvent::process() if (!success) { //Need to send on a retry cachePort->cshrRetry = pkt; + cachePort->waitingOnRetry = true; } else if (cachePort->cache->doSlaveRequest()) { @@ -255,12 +263,13 @@ BaseCache::CacheEvent::process() pkt->result = Packet::Success; pkt->makeTimingResponse(); if (!cachePort->drainList.empty()) { - //Already blocked waiting for bus, just append + //Already have a list, just append cachePort->drainList.push_back(pkt); } else if (!cachePort->sendTiming(pkt)) { //It failed, save it to list of drain events cachePort->drainList.push_back(pkt); + cachePort->waitingOnRetry = true; } } diff --git a/src/mem/cache/base_cache.hh b/src/mem/cache/base_cache.hh index e0f12940f..41c28f3a1 100644 --- a/src/mem/cache/base_cache.hh +++ b/src/mem/cache/base_cache.hh @@ -112,6 +112,8 @@ class BaseCache : public MemObject bool isCpuSide; + bool waitingOnRetry; + std::list drainList; Packet *cshrRetry; @@ -465,7 +467,7 @@ class BaseCache : public MemObject */ void setMasterRequest(RequestCause cause, Tick time) { - if (!doMasterRequest()) + if (!doMasterRequest() && memSidePort->drainList.empty()) { BaseCache::CacheEvent * reqCpu = new BaseCache::CacheEvent(memSidePort); reqCpu->schedule(time); @@ -527,6 +529,10 @@ class BaseCache : public MemObject CacheEvent *reqCpu = new CacheEvent(cpuSidePort, pkt); reqCpu->schedule(time); } + else { + if (pkt->cmd == Packet::Writeback) delete pkt->req; + delete pkt; + } } /** @@ -543,6 +549,10 @@ class BaseCache : public MemObject CacheEvent *reqCpu = new CacheEvent(cpuSidePort, pkt); reqCpu->schedule(time); } + else { + if (pkt->cmd == Packet::Writeback) delete pkt->req; + delete pkt; + } } /** diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index 8c0521b52..58eb0bdbc 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -193,19 +193,6 @@ Cache::access(PacketPtr &pkt) prefetcher->handleMiss(pkt, curTick); } if (!pkt->req->isUncacheable()) { - if (pkt->isInvalidate() && !pkt->isRead() - && !pkt->isWrite()) { - //Upgrade or Invalidate - //Look into what happens if two slave caches on bus - DPRINTF(Cache, "%s %x ? blk_addr: %x\n", pkt->cmdString(), - pkt->getAddr() & (((ULL(1))<<48)-1), - pkt->getAddr() & ~((Addr)blkSize - 1)); - - pkt->flags |= SATISFIED; - //Invalidates/Upgrades need no response if they get the bus -// return MA_HIT; //@todo, return values - return true; - } blk = tags->handleAccess(pkt, lat, writebacks); } else { size = pkt->getSize(); @@ -241,7 +228,10 @@ Cache::access(PacketPtr &pkt) // clear dirty bit if write through if (pkt->needsResponse()) respond(pkt, curTick+lat); -// return MA_HIT; + if (pkt->cmd == Packet::Writeback) { + //Signal that you can kill the pkt/req + pkt->flags |= SATISFIED; + } return true; } @@ -287,22 +277,22 @@ void Cache::sendResult(PacketPtr &pkt, MSHR* mshr, bool success) { if (success && !(pkt->flags & NACKED_LINE)) { - missQueue->markInService(pkt, mshr); - //Temp Hack for UPGRADES - if (pkt->cmd == Packet::UpgradeReq) { - pkt->flags &= ~CACHE_LINE_FILL; - BlkType *blk = tags->findBlock(pkt); - CacheBlk::State old_state = (blk) ? blk->status : 0; - CacheBlk::State new_state = coherence->getNewState(pkt,old_state); - DPRINTF(Cache, "Block for blk addr %x moving from state %i to %i\n", + missQueue->markInService(pkt, mshr); + //Temp Hack for UPGRADES + if (pkt->cmd == Packet::UpgradeReq) { + pkt->flags &= ~CACHE_LINE_FILL; + BlkType *blk = tags->findBlock(pkt); + CacheBlk::State old_state = (blk) ? blk->status : 0; + CacheBlk::State new_state = coherence->getNewState(pkt,old_state); + DPRINTF(Cache, "Block for blk addr %x moving from state %i to %i\n", pkt->getAddr() & (((ULL(1))<<48)-1), old_state, new_state); - //Set the state on the upgrade - memcpy(pkt->getPtr(), blk->data, blkSize); - PacketList writebacks; - tags->handleFill(blk, mshr, new_state, writebacks, pkt); - assert(writebacks.empty()); - missQueue->handleResponse(pkt, curTick + hitLatency); - } + //Set the state on the upgrade + memcpy(pkt->getPtr(), blk->data, blkSize); + PacketList writebacks; + tags->handleFill(blk, mshr, new_state, writebacks, pkt); + assert(writebacks.empty()); + missQueue->handleResponse(pkt, curTick + hitLatency); + } } else if (pkt && !pkt->req->isUncacheable()) { pkt->flags &= ~NACKED_LINE; pkt->flags &= ~SATISFIED; diff --git a/src/mem/cache/miss/mshr_queue.cc b/src/mem/cache/miss/mshr_queue.cc index 78897c8fb..1876a8987 100644 --- a/src/mem/cache/miss/mshr_queue.cc +++ b/src/mem/cache/miss/mshr_queue.cc @@ -215,6 +215,11 @@ MSHRQueue::markInService(MSHR* mshr) //assert(mshr == pendingList.front()); if (!(mshr->pkt->needsResponse() || mshr->pkt->cmd == Packet::UpgradeReq)) { assert(mshr->getNumTargets() == 0); + if ((mshr->pkt->flags & SATISFIED) && (mshr->pkt->cmd == Packet::Writeback)) { + //Writeback hit, so delete it + //otherwise the consumer will delete it + delete mshr->pkt->req; + } deallocate(mshr); return; } -- 2.30.2