From ac1368df50af123b32b41d7115ea4a0f15f7c97f Mon Sep 17 00:00:00 2001 From: Andreas Hansson Date: Fri, 6 Nov 2015 03:26:21 -0500 Subject: [PATCH] mem: Unify delayed packet deletion This patch unifies how we deal with delayed packet deletion, where the receiving slave is responsible for deleting the packet, but the sending agent (e.g. a cache) is still relying on the pointer until the call to sendTimingReq completes. Previously we used a mix of a deletion vector and a construct using unique_ptr. With this patch we ensure all slaves use the latter approach. --- src/mem/cache/cache.cc | 29 +++++++++-------------------- src/mem/cache/cache.hh | 7 +++---- src/mem/coherent_xbar.cc | 11 ++++------- src/mem/coherent_xbar.hh | 7 +++---- src/mem/dram_ctrl.cc | 10 ++-------- src/mem/dram_ctrl.hh | 8 ++++---- src/mem/dramsim2.cc | 13 +++---------- src/mem/dramsim2.hh | 8 ++++---- src/mem/simple_mem.cc | 10 ++-------- src/mem/simple_mem.hh | 8 ++++---- src/mem/tport.cc | 12 ++---------- src/mem/tport.hh | 8 +++----- 12 files changed, 43 insertions(+), 88 deletions(-) diff --git a/src/mem/cache/cache.cc b/src/mem/cache/cache.cc index 3e5ed42bc..aa95d5604 100644 --- a/src/mem/cache/cache.cc +++ b/src/mem/cache/cache.cc @@ -542,15 +542,6 @@ bool Cache::recvTimingReq(PacketPtr pkt) { DPRINTF(CacheTags, "%s tags: %s\n", __func__, tags->print()); -//@todo Add back in MemDebug Calls -// MemDebug::cacheAccess(pkt); - - - /// @todo temporary hack to deal with memory corruption issue until - /// 4-phase transactions are complete - for (int x = 0; x < pendingDelete.size(); x++) - delete pendingDelete[x]; - pendingDelete.clear(); assert(pkt->isRequest()); @@ -602,10 +593,9 @@ Cache::recvTimingReq(PacketPtr pkt) // main memory will delete the packet } - /// @todo nominally we should just delete the packet here, - /// however, until 4-phase stuff we can't because sending - /// cache is still relying on it. - pendingDelete.push_back(pkt); + // queue for deletion, as the sending cache is still relying + // on the packet + pendingDelete.reset(pkt); // no need to take any action in this particular cache as the // caches along the path to memory are allowed to keep lines @@ -678,12 +668,11 @@ Cache::recvTimingReq(PacketPtr pkt) // by access(), that calls accessBlock() function. cpuSidePort->schedTimingResp(pkt, request_time); } else { - /// @todo nominally we should just delete the packet here, - /// however, until 4-phase stuff we can't because sending cache is - /// still relying on it. If the block is found in access(), - /// CleanEvict and Writeback messages will be deleted here as - /// well. - pendingDelete.push_back(pkt); + // queue the packet for deletion, as the sending cache is + // still relying on it; if the block is found in access(), + // CleanEvict and Writeback messages will be deleted + // here as well + pendingDelete.reset(pkt); } } else { // miss @@ -754,7 +743,7 @@ Cache::recvTimingReq(PacketPtr pkt) // CleanEvicts corresponding to blocks which have outstanding // requests in MSHRs can be deleted here. if (pkt->cmd == MemCmd::CleanEvict) { - pendingDelete.push_back(pkt); + pendingDelete.reset(pkt); } else { DPRINTF(Cache, "%s coalescing MSHR for %s addr %#llx size %d\n", __func__, pkt->cmdString(), pkt->getAddr(), diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh index ec436201b..ae9e7e694 100644 --- a/src/mem/cache/cache.hh +++ b/src/mem/cache/cache.hh @@ -195,11 +195,10 @@ class Cache : public BaseCache const bool prefetchOnAccess; /** - * @todo this is a temporary workaround until the 4-phase code is committed. - * upstream caches need this packet until true is returned, so hold it for - * deletion until a subsequent call + * Upstream caches need this packet until true is returned, so + * hold it for deletion until a subsequent call */ - std::vector pendingDelete; + std::unique_ptr pendingDelete; /** * Does all the processing necessary to perform the provided request. diff --git a/src/mem/coherent_xbar.cc b/src/mem/coherent_xbar.cc index 6a0136ea8..223ab6ab5 100644 --- a/src/mem/coherent_xbar.cc +++ b/src/mem/coherent_xbar.cc @@ -140,12 +140,6 @@ CoherentXBar::init() bool CoherentXBar::recvTimingReq(PacketPtr pkt, PortID slave_port_id) { - // @todo temporary hack to deal with memory corruption issue until - // 4-phase transactions are complete - for (int x = 0; x < pendingDelete.size(); x++) - delete pendingDelete[x]; - pendingDelete.clear(); - // determine the source port based on the id SlavePort *src_port = slavePorts[slave_port_id]; @@ -223,7 +217,10 @@ CoherentXBar::recvTimingReq(PacketPtr pkt, PortID slave_port_id) // update the layer state and schedule an idle event reqLayers[master_port_id]->succeededTiming(packetFinishTime); - pendingDelete.push_back(pkt); + + // queue the packet for deletion + pendingDelete.reset(pkt); + return true; } diff --git a/src/mem/coherent_xbar.hh b/src/mem/coherent_xbar.hh index e99e9374f..24d00628b 100644 --- a/src/mem/coherent_xbar.hh +++ b/src/mem/coherent_xbar.hh @@ -274,11 +274,10 @@ class CoherentXBar : public BaseXBar const Cycles snoopResponseLatency; /** - * @todo this is a temporary workaround until the 4-phase code is committed. - * upstream caches need this packet until true is returned, so hold it for - * deletion until a subsequent call + * Upstream caches need this packet until true is returned, so + * hold it for deletion until a subsequent call */ - std::vector pendingDelete; + std::unique_ptr pendingDelete; /** Function called by the port when the crossbar is recieving a Timing request packet.*/ diff --git a/src/mem/dram_ctrl.cc b/src/mem/dram_ctrl.cc index 3ba4616f7..4bd83f761 100644 --- a/src/mem/dram_ctrl.cc +++ b/src/mem/dram_ctrl.cc @@ -586,12 +586,6 @@ DRAMCtrl::printQs() const { bool DRAMCtrl::recvTimingReq(PacketPtr pkt) { - /// @todo temporary hack to deal with memory corruption issues until - /// 4-phase transactions are complete - for (int x = 0; x < pendingDelete.size(); x++) - delete pendingDelete[x]; - pendingDelete.clear(); - // This is where we enter from the outside world DPRINTF(DRAM, "recvTimingReq: request %s addr %lld size %d\n", pkt->cmdString(), pkt->getAddr(), pkt->getSize()); @@ -600,7 +594,7 @@ DRAMCtrl::recvTimingReq(PacketPtr pkt) if (pkt->memInhibitAsserted() || pkt->cmd == MemCmd::CleanEvict) { DPRINTF(DRAM, "Inhibited packet or clean evict -- Dropping it now\n"); - pendingDelete.push_back(pkt); + pendingDelete.reset(pkt); return true; } @@ -872,7 +866,7 @@ DRAMCtrl::accessAndRespond(PacketPtr pkt, Tick static_latency) } else { // @todo the packet is going to be deleted, and the DRAMPacket // is still having a pointer to it - pendingDelete.push_back(pkt); + pendingDelete.reset(pkt); } DPRINTF(DRAM, "Done\n"); diff --git a/src/mem/dram_ctrl.hh b/src/mem/dram_ctrl.hh index 617c94914..6cd72b266 100644 --- a/src/mem/dram_ctrl.hh +++ b/src/mem/dram_ctrl.hh @@ -836,11 +836,11 @@ class DRAMCtrl : public AbstractMemory // timestamp offset uint64_t timeStampOffset; - /** @todo this is a temporary workaround until the 4-phase code is - * committed. upstream caches needs this packet until true is returned, so - * hold onto it for deletion until a subsequent call + /** + * Upstream caches need this packet until true is returned, so + * hold it for deletion until a subsequent call */ - std::vector pendingDelete; + std::unique_ptr pendingDelete; /** * This function increments the energy when called. If stats are diff --git a/src/mem/dramsim2.cc b/src/mem/dramsim2.cc index 64acc5b57..cd0f45b23 100644 --- a/src/mem/dramsim2.cc +++ b/src/mem/dramsim2.cc @@ -178,16 +178,10 @@ DRAMSim2::recvTimingReq(PacketPtr pkt) // we should never see a new request while in retry assert(!retryReq); - // @todo temporary hack to deal with memory corruption issues until - // 4-phase transactions are complete - for (int x = 0; x < pendingDelete.size(); x++) - delete pendingDelete[x]; - pendingDelete.clear(); - if (pkt->memInhibitAsserted()) { // snooper will supply based on copy of packet // still target's responsibility to delete packet - pendingDelete.push_back(pkt); + pendingDelete.reset(pkt); return true; } @@ -281,9 +275,8 @@ DRAMSim2::accessAndRespond(PacketPtr pkt) if (!retryResp && !sendResponseEvent.scheduled()) schedule(sendResponseEvent, time); } else { - // @todo the packet is going to be deleted, and the DRAMPacket - // is still having a pointer to it - pendingDelete.push_back(pkt); + // queue the packet for deletion + pendingDelete.reset(pkt); } } diff --git a/src/mem/dramsim2.hh b/src/mem/dramsim2.hh index e57479247..5cde19cd8 100644 --- a/src/mem/dramsim2.hh +++ b/src/mem/dramsim2.hh @@ -160,11 +160,11 @@ class DRAMSim2 : public AbstractMemory */ EventWrapper tickEvent; - /** @todo this is a temporary workaround until the 4-phase code is - * committed. upstream caches needs this packet until true is returned, so - * hold onto it for deletion until a subsequent call + /** + * Upstream caches need this packet until true is returned, so + * hold it for deletion until a subsequent call */ - std::vector pendingDelete; + std::unique_ptr pendingDelete; public: diff --git a/src/mem/simple_mem.cc b/src/mem/simple_mem.cc index f68066e75..7e350feb6 100644 --- a/src/mem/simple_mem.cc +++ b/src/mem/simple_mem.cc @@ -97,16 +97,10 @@ SimpleMemory::recvFunctional(PacketPtr pkt) bool SimpleMemory::recvTimingReq(PacketPtr pkt) { - /// @todo temporary hack to deal with memory corruption issues until - /// 4-phase transactions are complete - for (int x = 0; x < pendingDelete.size(); x++) - delete pendingDelete[x]; - pendingDelete.clear(); - if (pkt->memInhibitAsserted()) { // snooper will supply based on copy of packet // still target's responsibility to delete packet - pendingDelete.push_back(pkt); + pendingDelete.reset(pkt); return true; } @@ -165,7 +159,7 @@ SimpleMemory::recvTimingReq(PacketPtr pkt) if (!retryResp && !dequeueEvent.scheduled()) schedule(dequeueEvent, packetQueue.back().tick); } else { - pendingDelete.push_back(pkt); + pendingDelete.reset(pkt); } return true; diff --git a/src/mem/simple_mem.hh b/src/mem/simple_mem.hh index 35d8aeafb..d19de7608 100644 --- a/src/mem/simple_mem.hh +++ b/src/mem/simple_mem.hh @@ -175,11 +175,11 @@ class SimpleMemory : public AbstractMemory */ Tick getLatency() const; - /** @todo this is a temporary workaround until the 4-phase code is - * committed. upstream caches needs this packet until true is returned, so - * hold onto it for deletion until a subsequent call + /** + * Upstream caches need this packet until true is returned, so + * hold it for deletion until a subsequent call */ - std::vector pendingDelete; + std::unique_ptr pendingDelete; public: diff --git a/src/mem/tport.cc b/src/mem/tport.cc index aa783ada0..9f9403a22 100644 --- a/src/mem/tport.cc +++ b/src/mem/tport.cc @@ -62,12 +62,6 @@ SimpleTimingPort::recvFunctional(PacketPtr pkt) bool SimpleTimingPort::recvTimingReq(PacketPtr pkt) { - /// @todo temporary hack to deal with memory corruption issue until - /// 4-phase transactions are complete. Remove me later - for (int x = 0; x < pendingDelete.size(); x++) - delete pendingDelete[x]; - pendingDelete.clear(); - // the SimpleTimingPort should not be used anywhere where there is // a need to deal with inhibited packets if (pkt->memInhibitAsserted()) @@ -82,10 +76,8 @@ SimpleTimingPort::recvTimingReq(PacketPtr pkt) assert(pkt->isResponse()); schedTimingResp(pkt, curTick() + latency); } else { - /// @todo nominally we should just delete the packet here. - /// Until 4-phase stuff we can't because the sending - /// cache is still relying on it - pendingDelete.push_back(pkt); + // queue the packet for deletion + pendingDelete.reset(pkt); } return true; diff --git a/src/mem/tport.hh b/src/mem/tport.hh index 166a23125..d7e4bbc74 100644 --- a/src/mem/tport.hh +++ b/src/mem/tport.hh @@ -81,12 +81,10 @@ class SimpleTimingPort : public QueuedSlavePort virtual Tick recvAtomic(PacketPtr pkt) = 0; /** - * @todo this is a temporary workaround until the 4-phase code is committed. - * upstream caches need this packet until true is returned, so hold it for - * deletion until a subsequent call + * Upstream caches need this packet until true is returned, so + * hold it for deletion until a subsequent call */ - std::vector pendingDelete; - + std::unique_ptr pendingDelete; public: -- 2.30.2