> On 2012-01-06 17:49:56, Brad Beckmann wrote: > > src/mem/ruby/system/Sequencer.cc, line 526 > > <http://reviews.m5sim.org/r/927/diff/2/?file=16851#file16851line526> > > > > I like the term cooldown, but I think it is a little bit confusing in > > this situation. If I understand this code correctly, you're not trying to > > avoid data reads and writes before taking a checkpoint. Rather you don't > > want the hitcallbacks of Flush requests trying to modify memory. Is that > > correct? If so, I thought we already dealt with that problem by simply not > > setting the data pointer of flush packet. Maybe I'm missing something. > > Nilay Vaish wrote: > You are right that the data pointer of the flush packet is NULL. > But if this 'else if' clause is removed, then the 'else' clause > will be executed, and if debug flag MemoryAccess is enabled, > we will get some prints. > > As such I am ready to remove this 'else if' clause.
I think seeing the prints if the MemoryAccess debug flag is enabled is fine, but if you rather not see them for flush requests, I would add an explicit if not Flush request condition to make it more clear what you're trying to avoid here. Adding the cooldown flag is really confusing. > On 2012-01-06 17:49:56, Brad Beckmann wrote: > > src/mem/ruby/system/Sequencer.cc, line 564 > > <http://reviews.m5sim.org/r/927/diff/2/?file=16851#file16851line564> > > > > Am I reading this correctly? Is this issuing the next fetch or flush > > request inside the previous request's callback? Is that how the patch used > > to work? > > > > If so, that seems very odd and would create one deep call stack. I > > must be missing something here. Possibly the next request is being > > schedule for the next cycle. If that is the case, then these functions > > should be renamed to something like "notifyFetchCompletion" or > > "notifyFlushCompletion" > > Nilay Vaish wrote: > The flush/fetch request will get enqueued in the mandatory queue, which > will be taken serviced next time the controller wakes up. This will not > create a deep call stack. Actually the Cache Recorder is issuing these > fetch and flush requests. From that point of view, I feel > issueFlushRequest() > sounds better. > > But I have a question here. Can the flush request be generated for any > cache line? Or does that cache line needs to be in the L1 cache? I > expecting > that flush requests can be issued for any cache line, or else I am not > sure how > data from the L2 cache will get flushed to the memory. I still don't think the functions issueFlush/FetchRequest are named correctly. They definitely imply a deep call stack. How about enqueueNextFlush/FetchRequest instead? To answer your question, flush requests can be issued for any address in physical memory. You don't need to have to move data to the L1 to flush the L2, though underneath the protocol's transistions may implement the flush in that way. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/927/#review1863 ----------------------------------------------------------- On 2012-01-07 05:15:42, Nilay Vaish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/927/ > ----------------------------------------------------------- > > (Updated 2012-01-07 05:15:42) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > Ruby: Resurrect Cache Warmup Capability > This patch resurrects ruby's cache warmup capability. It essentially > makes use of all the infrastructure that was added to the controllers, > memories and the cache recorder. > > > Diffs > ----- > > src/mem/ruby/buffers/MessageBuffer.cc c3d878fbdaea > src/mem/ruby/system/DMASequencer.hh c3d878fbdaea > src/mem/ruby/system/DirectoryMemory.cc c3d878fbdaea > src/mem/ruby/system/RubyPort.hh c3d878fbdaea > src/mem/ruby/system/RubyPort.cc c3d878fbdaea > src/mem/ruby/system/Sequencer.hh c3d878fbdaea > src/mem/ruby/system/Sequencer.cc c3d878fbdaea > src/mem/ruby/system/System.hh c3d878fbdaea > src/mem/ruby/system/System.cc c3d878fbdaea > > Diff: http://reviews.m5sim.org/r/927/diff > > > Testing > ------- > > > Thanks, > > Nilay > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
