Well, I think the first step is understanding why the cached version of the virtual port in the thread context behaves differently than a newly created one. From reading the code it doesn't seem like there should be a difference, however using the cached version doesn't work so there must be.
Ali If we fixed that then we might not need On Jun 29, 2008, at 6:28 PM, nathan binkert wrote: > I'd actually like to see this changset go back in. It seems that all > that needs to happen is that setPeer should delete the peer if it is a > defaultPort. There are a couple of ways to do this: > 1) Have setPeer call disconnect(), though I'm not clear on how the > deletePort() thing is supposed to work > 2) in setPeer, dynamic_cast<DefaultPort *>(this) and delete it if it > isn't null > 3) create a new virtual function on Port that does nothing, and > override it to delete this in DefaultPort. Call this function from > setPeer. > > Anyway, Steve, since you're the most familiar with this code, I'd > prefer you put it back in, but I would be handy to have soon. > > Nate > > >> changeset 85c8d296c1cb in /z/repo/m5 >> details: http://repo.m5sim.org/m5?cmd=changeset;node=85c8d296c1cb >> summary: Backed out changeset 94a7bb476fca: caused memory leak. >> >> changeset 0d9fac06c402 in /z/repo/m5 >> details: http://repo.m5sim.org/m5?cmd=changeset;node=0d9fac06c402 >> summary: Automated merge after backout. >> >> diffstat: >> >> 11 files changed, 7 insertions(+), 17 deletions(-) >> src/cpu/o3/fetch.hh | 1 - >> src/cpu/o3/lsq.hh | 1 - >> src/mem/bus.cc | 1 - >> src/mem/cache/cache_impl.hh | 4 ++-- >> src/mem/mem_object.cc | 1 - >> src/mem/mem_object.hh | 3 --- >> src/mem/port.cc | 7 ------- >> src/mem/port.hh | 2 ++ >> src/python/swig/event.i | 1 - >> src/sim/sim_object.hh | 2 ++ >> src/sim/sim_object_params.hh | 1 + >> >> diffs (truncated from 453 to 300 lines): >> >> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/fetch.hh >> --- a/src/cpu/o3/fetch.hh Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/cpu/o3/fetch.hh Sat Jun 28 13:20:00 2008 -0400 >> @@ -80,8 +80,8 @@ class DefaultFetch >> >> public: >> /** Default constructor. */ >> - IcachePort(DefaultFetch<Impl> *_fetch, O3CPU *_cpu) >> - : Port(_fetch->name() + "-iport", _cpu), fetch(_fetch) >> + IcachePort(DefaultFetch<Impl> *_fetch) >> + : Port(_fetch->name() + "-iport"), fetch(_fetch) >> { } >> >> bool snoopRangeSent; >> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/fetch_impl.hh >> --- a/src/cpu/o3/fetch_impl.hh Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/cpu/o3/fetch_impl.hh Sat Jun 28 13:20:00 2008 -0400 >> @@ -167,7 +167,7 @@ DefaultFetch<Impl>::DefaultFetch(O3CPU * >> 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 -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/lsq.hh >> --- a/src/cpu/o3/lsq.hh Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/cpu/o3/lsq.hh Sat Jun 28 13:20:00 2008 -0400 >> @@ -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 -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/lsq_impl.hh >> --- a/src/cpu/o3/lsq_impl.hh Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/cpu/o3/lsq_impl.hh Sat Jun 28 13:20:00 2008 -0400 >> @@ -112,7 +112,7 @@ LSQ<Impl>::DcachePort::recvRetry() >> >> template <class Impl> >> LSQ<Impl>::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 -r 94a7bb476fca -r 0d9fac06c402 src/cpu/o3/ >> thread_context_impl.hh >> --- a/src/cpu/o3/thread_context_impl.hh Sat Jun 21 01:04:43 2008 >> -0400 >> +++ b/src/cpu/o3/thread_context_impl.hh Sat Jun 28 13:20:00 2008 >> -0400 >> @@ -103,6 +103,7 @@ O3ThreadContext<Impl>::delVirtPort(Virtu >> O3ThreadContext<Impl>::delVirtPort(VirtualPort *vp) >> { >> if (vp != thread->getVirtPort()) { >> + vp->removeConn(); >> delete vp; >> } >> } >> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/ozone/cpu_impl.hh >> --- a/src/cpu/ozone/cpu_impl.hh Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/cpu/ozone/cpu_impl.hh Sat Jun 28 13:20:00 2008 -0400 >> @@ -747,6 +747,7 @@ void >> void >> OzoneCPU<Impl>::OzoneTC::delVirtPort(VirtualPort *vp) >> { >> + vp->removeConn(); >> delete vp; >> } >> #endif >> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/simple_thread.cc >> --- a/src/cpu/simple_thread.cc Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/cpu/simple_thread.cc Sat Jun 28 13:20:00 2008 -0400 >> @@ -302,6 +302,7 @@ SimpleThread::delVirtPort(VirtualPort *v >> SimpleThread::delVirtPort(VirtualPort *vp) >> { >> if (vp != virtPort) { >> + vp->removeConn(); >> delete vp; >> } >> } >> diff -r 94a7bb476fca -r 0d9fac06c402 src/cpu/thread_state.cc >> --- a/src/cpu/thread_state.cc Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/cpu/thread_state.cc Sat Jun 28 13:20:00 2008 -0400 >> @@ -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 -r 94a7bb476fca -r 0d9fac06c402 src/mem/bridge.cc >> --- a/src/mem/bridge.cc Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/mem/bridge.cc Sat Jun 28 13:20:00 2008 -0400 >> @@ -47,7 +47,7 @@ Bridge::BridgePort::BridgePort(const std >> int _delay, int _nack_delay, int >> _req_limit, >> int _resp_limit, >> std::vector<Range<Addr> > >> 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 -r 94a7bb476fca -r 0d9fac06c402 src/mem/bus.cc >> --- a/src/mem/bus.cc Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/mem/bus.cc Sat Jun 28 13:20:00 2008 -0400 >> @@ -72,8 +72,8 @@ Bus::getPort(const std::string &if_name, >> return bp; >> } >> >> -bool >> -Bus::deletePort(Port *p) >> +void >> +Bus::deletePortRefs(Port *p) >> { >> >> BusPort *bp = dynamic_cast<BusPort*>(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. */ >> @@ -524,9 +523,12 @@ Bus::recvStatusChange(Port::Status statu >> for (iter = ranges.begin(); iter != ranges.end(); iter++) { >> DPRINTF(BusAddrRanges, "Adding range %#llx - %#llx for >> id %d\n", >> iter->start, iter->end, id); >> - if (portMap.insert(*iter, id) == portMap.end()) >> - panic("Two devices with same range\n"); >> - >> + if (portMap.insert(*iter, id) == portMap.end()) { >> + int conflict_id = portMap.find(*iter)->second; >> + fatal("%s has two ports with same range:\n\t%s\n\t >> %s\n", >> + name(), interfaces[id]->getPeer()->name(), >> + interfaces[conflict_id]->getPeer()->name()); >> + } >> } >> } >> DPRINTF(MMU, "port list has %d entries\n", portMap.size()); >> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/bus.hh >> --- a/src/mem/bus.hh Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/mem/bus.hh Sat Jun 28 13:20:00 2008 -0400 >> @@ -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 -r 94a7bb476fca -r 0d9fac06c402 src/mem/cache/cache.hh >> --- a/src/mem/cache/cache.hh Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/mem/cache/cache.hh Sat Jun 28 13:20:00 2008 -0400 >> @@ -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 -r 94a7bb476fca -r 0d9fac06c402 src/mem/cache/cache_impl.hh >> --- a/src/mem/cache/cache_impl.hh Sat Jun 21 01:04:43 2008 >> -0400 >> +++ b/src/mem/cache/cache_impl.hh Sat Jun 28 13:20:00 2008 >> -0400 >> @@ -105,14 +105,13 @@ Cache<TagStore>::getPort(const std::stri >> } >> >> template<class TagStore> >> -bool >> -Cache<TagStore>::deletePort(Port *p) >> +void >> +Cache<TagStore>::deletePortRefs(Port *p) >> { >> if (cpuSidePort == p || memSidePort == p) >> panic("Can only delete functional ports\n"); >> >> delete p; >> - return true; >> } >> >> >> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/mem_object.cc >> --- a/src/mem/mem_object.cc Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/mem/mem_object.cc Sat Jun 28 13:20:00 2008 -0400 >> @@ -43,8 +43,8 @@ MemObject::makeParams(const std::string >> return params; >> } >> >> -bool >> -MemObject::deletePort(Port *p) >> +void >> +MemObject::deletePortRefs(Port *p) >> { >> panic("This object does not support port deletion\n"); >> } >> diff -r 94a7bb476fca -r 0d9fac06c402 src/mem/mem_object.hh >> --- a/src/mem/mem_object.hh Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/mem/mem_object.hh Sat Jun 28 13:20:00 2008 -0400 >> @@ -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 -r 94a7bb476fca -r 0d9fac06c402 src/mem/port.cc >> --- a/src/mem/port.cc Sat Jun 21 01:04:43 2008 -0400 >> +++ b/src/mem/port.cc Sat Jun 28 13:20:00 2008 -0400 >> @@ -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() : >> "<unknown>"); >> + 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(const std::string &_name, MemObject *_owner) >> + : portName(_name), peer(&defaultPeerPort), owner(_owner) >> { >> } >> >> Port::~Port() >> { >> - disconnectFromPeer(); >> -} >> - >> -void >> -Port::disconnectFromPeer() >> -{ >> - if (peer) { >> _______________________________________________ >> m5-dev mailing list >> m5-dev@m5sim.org >> http://m5sim.org/mailman/listinfo/m5-dev >> >> > _______________________________________________ > m5-dev mailing list > m5-dev@m5sim.org > http://m5sim.org/mailman/listinfo/m5-dev > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
