changeset 0964165d1857 in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=0964165d1857
description:
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.
diffstat:
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(-)
diffs (truncated from 326 to 300 lines):
diff -r 4237221d3e31 -r 0964165d1857 src/mem/cache/cache.cc
--- a/src/mem/cache/cache.cc Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/cache/cache.cc Fri Nov 06 03:26:21 2015 -0500
@@ -542,15 +542,6 @@
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 @@
// 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 @@
// 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 @@
// 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 -r 4237221d3e31 -r 0964165d1857 src/mem/cache/cache.hh
--- a/src/mem/cache/cache.hh Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/cache/cache.hh Fri Nov 06 03:26:21 2015 -0500
@@ -195,11 +195,10 @@
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<PacketPtr> pendingDelete;
+ std::unique_ptr<Packet> pendingDelete;
/**
* Does all the processing necessary to perform the provided request.
diff -r 4237221d3e31 -r 0964165d1857 src/mem/coherent_xbar.cc
--- a/src/mem/coherent_xbar.cc Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/coherent_xbar.cc Fri Nov 06 03:26:21 2015 -0500
@@ -140,12 +140,6 @@
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 @@
// 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 -r 4237221d3e31 -r 0964165d1857 src/mem/coherent_xbar.hh
--- a/src/mem/coherent_xbar.hh Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/coherent_xbar.hh Fri Nov 06 03:26:21 2015 -0500
@@ -274,11 +274,10 @@
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<PacketPtr> pendingDelete;
+ std::unique_ptr<Packet> pendingDelete;
/** Function called by the port when the crossbar is recieving a Timing
request packet.*/
diff -r 4237221d3e31 -r 0964165d1857 src/mem/dram_ctrl.cc
--- a/src/mem/dram_ctrl.cc Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/dram_ctrl.cc Fri Nov 06 03:26:21 2015 -0500
@@ -586,12 +586,6 @@
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 @@
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 @@
} 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 -r 4237221d3e31 -r 0964165d1857 src/mem/dram_ctrl.hh
--- a/src/mem/dram_ctrl.hh Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/dram_ctrl.hh Fri Nov 06 03:26:21 2015 -0500
@@ -836,11 +836,11 @@
// 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<PacketPtr> pendingDelete;
+ std::unique_ptr<Packet> pendingDelete;
/**
* This function increments the energy when called. If stats are
diff -r 4237221d3e31 -r 0964165d1857 src/mem/dramsim2.cc
--- a/src/mem/dramsim2.cc Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/dramsim2.cc Fri Nov 06 03:26:21 2015 -0500
@@ -178,16 +178,10 @@
// 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 @@
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 -r 4237221d3e31 -r 0964165d1857 src/mem/dramsim2.hh
--- a/src/mem/dramsim2.hh Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/dramsim2.hh Fri Nov 06 03:26:21 2015 -0500
@@ -160,11 +160,11 @@
*/
EventWrapper<DRAMSim2, &DRAMSim2::tick> 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<PacketPtr> pendingDelete;
+ std::unique_ptr<Packet> pendingDelete;
public:
diff -r 4237221d3e31 -r 0964165d1857 src/mem/simple_mem.cc
--- a/src/mem/simple_mem.cc Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/simple_mem.cc Fri Nov 06 03:26:21 2015 -0500
@@ -97,16 +97,10 @@
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 @@
if (!retryResp && !dequeueEvent.scheduled())
schedule(dequeueEvent, packetQueue.back().tick);
} else {
- pendingDelete.push_back(pkt);
+ pendingDelete.reset(pkt);
}
return true;
diff -r 4237221d3e31 -r 0964165d1857 src/mem/simple_mem.hh
--- a/src/mem/simple_mem.hh Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/simple_mem.hh Fri Nov 06 03:26:21 2015 -0500
@@ -175,11 +175,11 @@
*/
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<PacketPtr> pendingDelete;
+ std::unique_ptr<Packet> pendingDelete;
public:
diff -r 4237221d3e31 -r 0964165d1857 src/mem/tport.cc
--- a/src/mem/tport.cc Fri Nov 06 03:26:16 2015 -0500
+++ b/src/mem/tport.cc Fri Nov 06 03:26:21 2015 -0500
@@ -62,12 +62,6 @@
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 @@
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
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
