I think someone just needs to reapply it and test it. Steve
On Mon, Jul 7, 2008 at 9:11 PM, nathan binkert <[EMAIL PROTECTED]> wrote: > 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 > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
