> On May 12, 2015, 3:24 p.m., Jason Power wrote: > > src/mem/ruby/system/RubyPort.cc, line 298 > > <http://reviews.gem5.org/r/2776/diff/1/?file=45140#file45140line298> > > > > I don't understand why the random tester doesn't want to retry failed > > requests. Could you explain why this feature is necessary? It's been a few > > years since I've looked deeply at the random tester. I'll hold off on > > broadcasting my opinion on this until I understand it better :). > > > > Again, this may be more appropriate as a separate patch, but I'm not > > going to push hard on this. > > Brad Beckmann wrote: > The ruby tester was never designed to handle failed request. The ruby > tester is designed to send large amount of racey requests to stress the > protocol logic. It does this by avoiding flow control and retries. > > Jason Power wrote: > Ok, I understand this a little better now. One more question, though. Why > is it that the RubyPort must be set to ignore retries and it's not the > responsibility of the tester to ignore retries? I think this may be just an > issue with the way the RubyPort is designed, but I'm trying to get a good > understanding of it. > > Overall, it's not very important to me. This code is only used when > testing, and it's explicitly noted in the code. So if it's difficult to move > this logic into the tester instead of the port this code is OK with me. > > Brad Beckmann wrote: > The RubyTester will not work if there is a retry. Every request it > issues needs to succeed. Therefore it doesn't make sense for the RubyTester > to ignore a retry. Instead it will panic...that is how it is currently > implemented. > > Jason Power wrote: > I think my issue here is very similar to Andreas's above. I believe that > a better solution than making the change in the RubyPort is to instead make > the change in the RubyTester. You're right, the RubyTester will panic, but > what is your reason for not changing the RubyTester to just ignore retries? > It seems simple to just remove the panic in line 79(ish) of RubyTester.hh. > > I would much rather see changes to the RubyTester to fix this problem > instead of changes to RubyPort. Changes to the RubyPort are much more > invasive than changes to the tester. Is it impossible to fix this in the > RubyTester? > > Brad Beckmann wrote: > The RubyTester will not work if it gets a retry. It is not as simple as > you suggest. The RubyTester is designed to stress the memory system. It is > not designed to throttle back it's requests. That defeats the purpose of the > RubyTester. > > The RubyTester is not broken and it does not need to be fixed. The > RubyPort needs to allow large bursts of requests if we are going to stress > the protocol logic. > > I would agrue these changes to the RubyPort are pretty minimal. They > just provide more flexibility of when and how it handles retries. Eventually > we will post additional patches that build on this flexibility.
Also I want to point out that the issue with how the RubyTester handles retries is not related to Andreas's question about increasing the number of buffered packets in the packet_queue. The retry question deals with how the RubyTester generates requests. The invisible packet limit is reached on the response path from the RubyPort back to the RubyTester. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2776/#review6171 ----------------------------------------------------------- On May 11, 2015, 10:28 p.m., Tony Gutierrez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2776/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 10:28 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10833:e624796bae17 > --------------------------- > ruby: cleaner ruby tester support > > This patch allows the ruby random tester to use ruby ports that may only > support instr or data requests. This patch is similar to a previous changeset > (8932:1b2c17565ac8) that was unfortunately broken by subsequent changesets. > This current patch implements the support in a more straight-forward way. > The patch also includes better DPRINTFs and generalizes the retry behavior > needed by the ruby tester so that other testers/cpu models can use it as well. > > > Diffs > ----- > > configs/example/ruby_random_test.py > fbdaa08aaa426b9f4660c366f934ccb670d954ec > configs/ruby/MESI_Three_Level.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > configs/ruby/MESI_Two_Level.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > configs/ruby/MI_example.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > configs/ruby/MOESI_CMP_directory.py > fbdaa08aaa426b9f4660c366f934ccb670d954ec > configs/ruby/MOESI_CMP_token.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > configs/ruby/MOESI_hammer.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/cpu/testers/rubytest/Check.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/cpu/testers/rubytest/CheckTable.cc > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/cpu/testers/rubytest/RubyTester.hh > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/cpu/testers/rubytest/RubyTester.cc > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/cpu/testers/rubytest/RubyTester.py > fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/packet_queue.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/ruby/system/RubyPort.hh fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/ruby/system/RubyPort.cc fbdaa08aaa426b9f4660c366f934ccb670d954ec > src/mem/ruby/system/Sequencer.py fbdaa08aaa426b9f4660c366f934ccb670d954ec > > Diff: http://reviews.gem5.org/r/2776/diff/ > > > Testing > ------- > > > Thanks, > > Tony Gutierrez > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
