From caaac16803db6eaf3ee20b5d062ec2211fe6584d Mon Sep 17 00:00:00 2001 From: Steve Reinhardt Date: Sat, 28 Jun 2008 13:19:38 -0400 Subject: [PATCH] Backed out changeset 94a7bb476fca: caused memory leak. --- src/cpu/o3/fetch.hh | 4 +- src/cpu/o3/fetch_impl.hh | 2 +- src/cpu/o3/lsq.hh | 4 +- src/cpu/o3/lsq_impl.hh | 2 +- src/cpu/o3/thread_context_impl.hh | 1 + src/cpu/ozone/cpu_impl.hh | 1 + src/cpu/simple_thread.cc | 1 + src/cpu/thread_state.cc | 4 +- src/mem/bridge.cc | 2 +- src/mem/bus.cc | 7 ++-- src/mem/bus.hh | 2 +- src/mem/cache/cache.hh | 2 +- src/mem/cache/cache_impl.hh | 5 +-- src/mem/mem_object.cc | 4 +- src/mem/mem_object.hh | 10 ++--- src/mem/port.cc | 65 ++++++++----------------------- src/mem/port.hh | 22 ++++++----- 17 files changed, 54 insertions(+), 84 deletions(-) diff --git a/src/cpu/o3/fetch.hh b/src/cpu/o3/fetch.hh index 3ada0328f..d954bd1e7 100644 --- a/src/cpu/o3/fetch.hh +++ b/src/cpu/o3/fetch.hh @@ -80,8 +80,8 @@ class DefaultFetch public: /** Default constructor. */ - IcachePort(DefaultFetch *_fetch, O3CPU *_cpu) - : Port(_fetch->name() + "-iport", _cpu), fetch(_fetch) + IcachePort(DefaultFetch *_fetch) + : Port(_fetch->name() + "-iport"), fetch(_fetch) { } bool snoopRangeSent; diff --git a/src/cpu/o3/fetch_impl.hh b/src/cpu/o3/fetch_impl.hh index ecfbacd98..7d344fa33 100644 --- a/src/cpu/o3/fetch_impl.hh +++ b/src/cpu/o3/fetch_impl.hh @@ -167,7 +167,7 @@ DefaultFetch::DefaultFetch(O3CPU *_cpu, Params *params) instSize = sizeof(TheISA::MachInst); // Name is finally available, so create the port. - icachePort = new IcachePort(this, cpu); + icachePort = new IcachePort(this); icachePort->snoopRangeSent = false; diff --git a/src/cpu/o3/lsq.hh b/src/cpu/o3/lsq.hh index 82d73c6ee..06de608e0 100644 --- a/src/cpu/o3/lsq.hh +++ b/src/cpu/o3/lsq.hh @@ -296,8 +296,8 @@ class LSQ { public: /** Default constructor. */ - DcachePort(LSQ *_lsq, O3CPU *_cpu) - : Port(_lsq->name() + "-dport", _cpu), lsq(_lsq) + DcachePort(LSQ *_lsq) + : Port(_lsq->name() + "-dport"), lsq(_lsq) { } bool snoopRangeSent; diff --git a/src/cpu/o3/lsq_impl.hh b/src/cpu/o3/lsq_impl.hh index 41ab66dd0..8ed6f7f54 100644 --- a/src/cpu/o3/lsq_impl.hh +++ b/src/cpu/o3/lsq_impl.hh @@ -112,7 +112,7 @@ LSQ::DcachePort::recvRetry() template LSQ::LSQ(O3CPU *cpu_ptr, IEW *iew_ptr, Params *params) - : cpu(cpu_ptr), iewStage(iew_ptr), dcachePort(this, cpu_ptr), + : cpu(cpu_ptr), iewStage(iew_ptr), dcachePort(this), LQEntries(params->LQEntries), SQEntries(params->SQEntries), numThreads(params->numberOfThreads), diff --git a/src/cpu/o3/thread_context_impl.hh b/src/cpu/o3/thread_context_impl.hh index 514d3de64..865d58635 100755 --- a/src/cpu/o3/thread_context_impl.hh +++ b/src/cpu/o3/thread_context_impl.hh @@ -103,6 +103,7 @@ void O3ThreadContext::delVirtPort(VirtualPort *vp) { if (vp != thread->getVirtPort()) { + vp->removeConn(); delete vp; } } diff --git a/src/cpu/ozone/cpu_impl.hh b/src/cpu/ozone/cpu_impl.hh index 4cb55fa23..0c7105382 100644 --- a/src/cpu/ozone/cpu_impl.hh +++ b/src/cpu/ozone/cpu_impl.hh @@ -747,6 +747,7 @@ template void OzoneCPU::OzoneTC::delVirtPort(VirtualPort *vp) { + vp->removeConn(); delete vp; } #endif diff --git a/src/cpu/simple_thread.cc b/src/cpu/simple_thread.cc index 47b69d05f..5a5444de4 100644 --- a/src/cpu/simple_thread.cc +++ b/src/cpu/simple_thread.cc @@ -302,6 +302,7 @@ void SimpleThread::delVirtPort(VirtualPort *vp) { if (vp != virtPort) { + vp->removeConn(); delete vp; } } diff --git a/src/cpu/thread_state.cc b/src/cpu/thread_state.cc index 56839ca7f..bcfc9c924 100644 --- a/src/cpu/thread_state.cc +++ b/src/cpu/thread_state.cc @@ -126,7 +126,7 @@ ThreadState::connectPhysPort() // already existed. Fix this memory leak once the bus port IDs // for functional ports is resolved. if (physPort) - physPort->disconnectFromPeer(); + physPort->removeConn(); else physPort = new FunctionalPort(csprintf("%s-%d-funcport", baseCpu->name(), tid)); @@ -140,7 +140,7 @@ ThreadState::connectVirtPort() // already existed. Fix this memory leak once the bus port IDs // for functional ports is resolved. if (virtPort) - virtPort->disconnectFromPeer(); + virtPort->removeConn(); else virtPort = new VirtualPort(csprintf("%s-%d-vport", baseCpu->name(), tid)); diff --git a/src/mem/bridge.cc b/src/mem/bridge.cc index 25c6e5d19..c09cacc00 100644 --- a/src/mem/bridge.cc +++ b/src/mem/bridge.cc @@ -47,7 +47,7 @@ Bridge::BridgePort::BridgePort(const std::string &_name, int _delay, int _nack_delay, int _req_limit, int _resp_limit, std::vector > filter_ranges) - : Port(_name, _bridge), bridge(_bridge), otherPort(_otherPort), + : Port(_name), bridge(_bridge), otherPort(_otherPort), delay(_delay), nackDelay(_nack_delay), filterRanges(filter_ranges), outstandingResponses(0), queuedRequests(0), inRetry(false), reqQueueLimit(_req_limit), respQueueLimit(_resp_limit), sendEvent(this) diff --git a/src/mem/bus.cc b/src/mem/bus.cc index 9dd3353fd..3bf1c6cfc 100644 --- a/src/mem/bus.cc +++ b/src/mem/bus.cc @@ -72,8 +72,8 @@ Bus::getPort(const std::string &if_name, int idx) return bp; } -bool -Bus::deletePort(Port *p) +void +Bus::deletePortRefs(Port *p) { BusPort *bp = dynamic_cast(p); @@ -81,11 +81,10 @@ Bus::deletePort(Port *p) panic("Couldn't convert Port* to BusPort*\n"); // If this is our one functional port if (funcPort == bp) - return false; + return; interfaces.erase(bp->getId()); clearBusCache(); delete bp; - return true; } /** Get the ranges of anyone other buses that we are connected to. */ diff --git a/src/mem/bus.hh b/src/mem/bus.hh index 7d476bb65..74901d626 100644 --- a/src/mem/bus.hh +++ b/src/mem/bus.hh @@ -364,7 +364,7 @@ class Bus : public MemObject /** A function used to return the port associated with this bus object. */ virtual Port *getPort(const std::string &if_name, int idx = -1); - virtual bool deletePort(Port *p); + virtual void deletePortRefs(Port *p); virtual void init(); virtual void startup(); diff --git a/src/mem/cache/cache.hh b/src/mem/cache/cache.hh index 56ae811a3..88c9deb7b 100644 --- a/src/mem/cache/cache.hh +++ b/src/mem/cache/cache.hh @@ -217,7 +217,7 @@ class Cache : public BaseCache Cache(const Params *p, TagStore *tags, BasePrefetcher *prefetcher); virtual Port *getPort(const std::string &if_name, int idx = -1); - virtual bool deletePort(Port *p); + virtual void deletePortRefs(Port *p); void regStats(); diff --git a/src/mem/cache/cache_impl.hh b/src/mem/cache/cache_impl.hh index a3dae7b2a..3b56c0a2e 100644 --- a/src/mem/cache/cache_impl.hh +++ b/src/mem/cache/cache_impl.hh @@ -105,14 +105,13 @@ Cache::getPort(const std::string &if_name, int idx) } template -bool -Cache::deletePort(Port *p) +void +Cache::deletePortRefs(Port *p) { if (cpuSidePort == p || memSidePort == p) panic("Can only delete functional ports\n"); delete p; - return true; } diff --git a/src/mem/mem_object.cc b/src/mem/mem_object.cc index 7d27db763..ce2a1107e 100644 --- a/src/mem/mem_object.cc +++ b/src/mem/mem_object.cc @@ -43,8 +43,8 @@ MemObject::makeParams(const std::string &name) return params; } -bool -MemObject::deletePort(Port *p) +void +MemObject::deletePortRefs(Port *p) { panic("This object does not support port deletion\n"); } diff --git a/src/mem/mem_object.hh b/src/mem/mem_object.hh index a91ea5ac4..33b56dfd4 100644 --- a/src/mem/mem_object.hh +++ b/src/mem/mem_object.hh @@ -64,13 +64,9 @@ class MemObject : public SimObject /** Additional function to return the Port of a memory object. */ virtual Port *getPort(const std::string &if_name, int idx = -1) = 0; - /** Tell MemObject that this port is no longer in use, so it - * should remove it from any structures that it's keeping it in. - * If the port was allocated dynamically for this connection, it - * should be deleted here. - * @return True if the port was deleted, false if it still exists. - */ - virtual bool deletePort(Port *p); + /** Tell object that this port is about to disappear, so it should remove it + * from any structures that it's keeping it in. */ + virtual void deletePortRefs(Port *p) ; }; #endif //__MEM_MEM_OBJECT_HH__ diff --git a/src/mem/port.cc b/src/mem/port.cc index 1e6989750..0e03194c9 100644 --- a/src/mem/port.cc +++ b/src/mem/port.cc @@ -39,25 +39,17 @@ #include "mem/mem_object.hh" #include "mem/port.hh" -/** - * Special class for port objects that are used as peers for - * unconnected ports. Assigning instances of this class to newly - * allocated ports allows us to guarantee that every port has a peer - * object (so there's no need to check for null peer pointers), while - * catching uses of unconnected ports. - */ class DefaultPeerPort : public Port { protected: void blowUp() { - Port *peer = getPeer(); - fatal("unconnected port: %s", peer ? peer->name() : ""); + fatal("%s: Unconnected port!", peer->name()); } public: - DefaultPeerPort(Port *_peer) - : Port("default_port", NULL, _peer) + DefaultPeerPort() + : Port("default_port") { } bool recvTiming(PacketPtr) @@ -96,59 +88,36 @@ class DefaultPeerPort : public Port bool isDefaultPort() const { return true; } }; +DefaultPeerPort defaultPeerPort; -Port::Port(const std::string &_name, MemObject *_owner, Port *_peer) : - portName(_name), - peer(_peer ? _peer : new DefaultPeerPort(this)), - owner(_owner) +Port::Port() + : peer(&defaultPeerPort), owner(NULL) { } -Port::~Port() +Port::Port(const std::string &_name, MemObject *_owner) + : portName(_name), peer(&defaultPeerPort), owner(_owner) { - disconnectFromPeer(); } -void -Port::disconnectFromPeer() +Port::~Port() { - if (peer) { - assert(peer->getPeer() == this); - peer->disconnect(); - } } void -Port::disconnect() +Port::setPeer(Port *port) { - // This notification should come only from our peer, so we must - // have one, - assert(peer != NULL); - // We must clear 'peer' here, else if owner->deletePort() calls - // delete on us then we'll recurse infinitely through the Port - // destructor. - peer = NULL; - // If owner->deletePort() returns true, then we've been deleted, - // so don't do anything but get out of here. If not, reset peer - // pointer to a DefaultPeerPort. - if (!(owner && owner->deletePort(this))) - peer = new DefaultPeerPort(this); + DPRINTF(Config, "setting peer to %s\n", port->name()); + + peer = port; } void -Port::setPeer(Port *port) +Port::removeConn() { - DPRINTF(Config, "setting peer to %s, old peer %s\n", - port->name(), peer ? peer->name() : ""); - - // You'd think we'd want to disconnect from the previous peer - // here, but it turns out that with some functional ports the old - // peer keeps using the connection, and it works because - // functional ports are unidirectional. - // - // disconnectFromPeer(); - - peer = port; + if (peer->getOwner()) + peer->getOwner()->deletePortRefs(peer); + peer = NULL; } void diff --git a/src/mem/port.hh b/src/mem/port.hh index 4e0d91e75..15fda2164 100644 --- a/src/mem/port.hh +++ b/src/mem/port.hh @@ -87,14 +87,17 @@ class Port public: + Port(); + /** * Constructor. * * @param _name Port name for DPRINTF output. Should include name * of memory system object to which the port belongs. * @param _owner Pointer to the MemObject that owns this port. + * Will not necessarily be set. */ - Port(const std::string &_name, MemObject *_owner, Port *_peer = NULL); + Port(const std::string &_name, MemObject *_owner = NULL); /** Return port name (for DPRINTF). */ const std::string &name() const { return portName; } @@ -108,18 +111,24 @@ class Port RangeChange }; + void setName(const std::string &name) + { portName = name; } + /** Function to set the pointer for the peer port. */ virtual void setPeer(Port *port); /** Function to get the pointer to the peer port. */ Port *getPeer() { return peer; } + /** Function to set the owner of this port. */ + void setOwner(MemObject *_owner) { owner = _owner; } + /** Function to return the owner of this port. */ MemObject *getOwner() { return owner; } - /** Notify my peer port that I'm disconnecting (by calling its - * disconnect() method) so it can clean up. */ - void disconnectFromPeer(); + /** Inform the peer port to delete itself and notify it's owner about it's + * demise. */ + void removeConn(); virtual bool isDefaultPort() const { return false; } @@ -245,11 +254,6 @@ class Port /** Internal helper function for read/writeBlob(). */ void blobHelper(Addr addr, uint8_t *p, int size, MemCmd cmd); - - /** Receive notification that my peer is disconnecting and clean - * up (potentially deleting myself in the process). Should be - * called only from peer's disconnectFromPeer(). */ - void disconnect(); }; /** A simple functional port that is only meant for one way communication to -- 2.30.2