----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/552/#review958 -----------------------------------------------------------
Hi Somayeh, I just have one larger issue to mention, then a few minor comments. See below for details. The larger issue is that I believe one of the transitions you specify is unecessary (see specific comments below). In general, be very careful about stalling the forwarded request queue. That usually leads to deadlock. To answer your questions: Yes, it does not make sense for the flush requests to return data. However, I don't think you want to add a new hitCallback function just for flushes. Instead, it seems that if you set the data pointer to null, then Ruby won't actually do any data updates. Subsequently, the function RubyPort::M5Port::hitCallback should add a condition that says if a flush request, set accessPhysMem to false. Make sense? Finally, yes please update Check.cc to not deal with data. Flush reqeusts should not replace Checks (i.e. reads). Instead they should be inserted randomly into the request stream. Once you make those changes, please post your next patch. I suspect that will be your final round of updates. src/mem/protocol/MOESI_hammer-cache.sm <http://reviews.m5sim.org/r/552/#comment1334> Can these transitions even happen? If I understand the protocol correctly, I don't think so because the directory is already blocked once one enters the MI_F state. Stalling the forward request network can lead to deadlock. In this particular case, I don't think it will because these transitions aren't actually executed, but in general this is bad practice. I know the L2 to L1 transfers stall the forward request network, but that is a special case to model transfer latency. Please either describe exactly how this transition is encountered or remove it. Thanks! src/mem/protocol/MOESI_hammer-cache.sm <http://reviews.m5sim.org/r/552/#comment1333> What does the "///???" mean? If I recall, the NC_DMA_GETS is an action used by uniprocessor systems. Does that answer your question? src/mem/ruby/system/Sequencer.cc <http://reviews.m5sim.org/r/552/#comment1335> missing space after } and before { src/mem/ruby/system/Sequencer.cc <http://reviews.m5sim.org/r/552/#comment1336> move '+' to previous line and align m_flush... with equal sign. - Brad On 2011-03-13 19:50:32, Somayeh Sardashti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/552/ > ----------------------------------------------------------- > > (Updated 2011-03-13 19:50:32) > > > Review request for Default and Brad Beckmann. > > > Summary > ------- > > my initial implementation of cache flushing > > > Diffs > ----- > > src/cpu/testers/rubytest/Check.hh baf4b5f6782e > src/cpu/testers/rubytest/Check.cc baf4b5f6782e > src/mem/packet.hh baf4b5f6782e > src/mem/packet.cc baf4b5f6782e > src/mem/protocol/MOESI_hammer-cache.sm baf4b5f6782e > src/mem/protocol/MOESI_hammer-dir.sm baf4b5f6782e > src/mem/protocol/MOESI_hammer-msg.sm baf4b5f6782e > src/mem/protocol/RubySlicc_Exports.sm baf4b5f6782e > src/mem/protocol/RubySlicc_Types.sm baf4b5f6782e > src/mem/ruby/slicc_interface/RubyRequest.hh baf4b5f6782e > src/mem/ruby/slicc_interface/RubyRequest.cc baf4b5f6782e > src/mem/ruby/system/DMASequencer.cc baf4b5f6782e > src/mem/ruby/system/RubyPort.cc baf4b5f6782e > src/mem/ruby/system/Sequencer.hh baf4b5f6782e > src/mem/ruby/system/Sequencer.cc baf4b5f6782e > > Diff: http://reviews.m5sim.org/r/552/diff > > > Testing > ------- > > This implementation has passed 10,000,000 ruby test cases, and the > MOESI_hammer regression tests. > > > Thanks, > > Somayeh > > _______________________________________________ m5-dev mailing list m5-dev@m5sim.org http://m5sim.org/mailman/listinfo/m5-dev
