mem: Delay deleting of incoming packets by one call.

This patch is a temporary fix until Andreas' four-phase patches
get reviewed and committed. Removing FastAlloc seems to have exposed
an issue which previously was reasonable rare in which packets are freed
before the sending cache is done with them. This change puts incoming packets
no a pendingDelete queue which are deleted at the start of the next call and
thus breaks the dependency between when the caller returns true and when the
packet is actually used by the sending cache.

Running valgrind on a multi-core linux boot and the memtester results in no
valgrind warnings.

diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh
index 04421b1e510474d8ac7577e67fa19b42bf8b7bab..beb3903daf8065e2065342459ecd6cdc3271c067 100644 (file)
--- a/src/mem/cache/cache.hh
+++ b/src/mem/cache/cache.hh
@@ -190,6 +190,13 @@ 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
+     */
+    std::vector<PacketPtr> pendingDelete;
+
     /**
      * Does all the processing necessary to perform the provided request.
      * @param pkt The memory request to perform.

diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh
index 942ac59ec6d4770e27a866ec7105d8098a6464ea..fec0a622232bb8846e4032637dec6951e5743b6e 100644 (file)
--- a/src/mem/cache/cache_impl.hh
+++ b/src/mem/cache/cache_impl.hh
@@ -378,6 +378,13 @@ Cache<TagStore>::timingAccess(PacketPtr pkt)
 //@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();
+
     // we charge hitLatency for doing just about anything here
     Tick time =  curTick() + hitLatency;
 
@@ -421,7 +428,11 @@ Cache<TagStore>::timingAccess(PacketPtr pkt)
         }
         // since we're the official target but we aren't responding,
         // delete the packet now.
-        delete pkt;
+
+        /// @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);
         return true;
     }
 
@@ -489,7 +500,10 @@ Cache<TagStore>::timingAccess(PacketPtr pkt)
             pkt->makeTimingResponse();
             cpuSidePort->respond(pkt, curTick()+lat);
         } else {
-            delete pkt;
+            /// @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);
         }
     } else {
         // miss

diff --git a/src/mem/tport.cc b/src/mem/tport.cc
index c071ef18c9ca3c94cd12a7aabbcebddaa15a4306..1ce3b4dc2e7b83cc11f2653db1f504ed6a2e0cad 100644 (file)
--- a/src/mem/tport.cc
+++ b/src/mem/tport.cc
@@ -62,6 +62,12 @@ 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();
+
     if (pkt->memInhibitAsserted()) {
         // snooper will supply based on copy of packet
         // still target's responsibility to delete packet
@@ -78,7 +84,10 @@ SimpleTimingPort::recvTimingReq(PacketPtr pkt)
         assert(pkt->isResponse());
         queue.schedSendTiming(pkt, curTick() + latency);
     } else {
-        delete pkt;
+        /// @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);
     }
 
     return true;

diff --git a/src/mem/tport.hh b/src/mem/tport.hh
index db5a074fbaea3cfe97760c34a33136cc77412e9a..1f08d1a912a137e37c8ea0d5dc07b5031aacc26b 100644 (file)
--- a/src/mem/tport.hh
+++ b/src/mem/tport.hh
@@ -73,6 +73,14 @@ 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
+     */
+    std::vector<PacketPtr> pendingDelete;
+
+
   public:
 
     /**