So, you committed that fix to the dynamic creating of virtual ports. What's the status of this diff?
> 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 > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
