> On Aug. 1, 2012, 3:51 p.m., Ali Saidi wrote: > > src/arch/arm/table_walker.cc, line 89 > > <http://reviews.gem5.org/r/1221/diff/7/?file=28379#file28379line89> > > > > you could do count + 1 here along with a comment that we're returning > > the count of the port + 1 because we still have stuff to do. > > Anthony Gutierrez wrote: > Also, recently I was thinking should we do something like: > > else { > if (count) { > changeState(Draining); > return count; > } > changeState(Drained); > return 0; > } > > In the case the table walker itself is drained, but its ports aren't? > Should it be "drained" if its ports are not? > > Ali Saidi wrote: > no it shouldn't be... if there is something in the port still, it's going > to be getting a callback to deal with it. > > Anthony Gutierrez wrote: > Ok. My reasoning for this was that I think any object that is responsible > for calling drain() on other objects, should first call drain() on the > objects it's responsible for again before it processes its drain event. E.g., > for the table walker we should do for completeDrain(): > > unsigned int count = port.drain(de); > > if (drainEvent && stateQueueL1.empty() && stateQueueL2.empty() && > pendingQueue.empty() && !count) { > // continue to set Drained and process drain event > > I tried this and it didn't seem to get me any further or fix any bugs, > but I still think it's correct. > > Also the cache and DmaDevice set their state to Draining if their ports > aren't drained, even though they themselves never contribute to count. Caches > also set the drainEvent when they're draining and never set it to NULL, i.e., > there is nothing like a completeDrain() for them. Either they don't need to > do that, or they themselves should have to drain somehow and not just their > ports. Currently, caches and DmaDevices are always drained if their ports are > drained.
It isn't doing that? I think it probably should. The caches don't do anything because they're not responsible for originating any requests (although caches with a prefetcher might violate this assumption). The device or a cpu is asking the cache to do a request on behalf of it, so if the device or cpu aren't drained the cache can't be. - Ali ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/1221/#review3187 ----------------------------------------------------------- On Aug. 2, 2012, 2:35 p.m., Anthony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/1221/ > ----------------------------------------------------------- > > (Updated Aug. 2, 2012, 2:35 p.m.) > > > Review request for Default. > > > Description > ------- > > Changeset 9139:65fc52d74391 > --------------------------- > O3,ARM: fix some problems with drain/switchout functionality and add Drain > DPRINTFs > > This patch fixes some problems with the drain/switchout functionality > for the O3 cpu and for the ARM ISA and adds some useful debug print > statements. > > This is an incremental fix as there are still a few bugs/mem leaks with the > switchout code. Particularly when switching from an O3CPU to a > TimingSimpleCPU. However, when switching from O3 to O3 cores with the ARM ISA > I haven't encountered any more assertion failures; now the kernel will > typically panic inside of simulation. > > > Diffs > ----- > > src/arch/arm/table_walker.hh b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/arch/arm/table_walker.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/cpu/base.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/cpu/o3/commit_impl.hh b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/cpu/o3/cpu.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/cpu/o3/fetch_impl.hh b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/cpu/o3/lsq_unit.hh b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/cpu/simple/timing.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/dev/copy_engine.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/dev/dma_device.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/dev/i8254xGBe.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/mem/bus.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/mem/cache/base.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/mem/packet_queue.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/mem/port.hh b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/mem/port.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/mem/ruby/system/RubyPort.cc b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/python/m5/simulate.py b4d0bdb5269420e4675d0aceff6f1392094a73f1 > src/sim/SConscript b4d0bdb5269420e4675d0aceff6f1392094a73f1 > > Diff: http://reviews.gem5.org/r/1221/diff/ > > > Testing > ------- > > > Thanks, > > Anthony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
