> On May 12, 2015, 5:45 a.m., Andreas Hansson wrote: > > src/mem/packet_queue.cc, line 117 > > <http://reviews.gem5.org/r/2776/diff/1/?file=45138#file45138line117> > > > > I strongly disagree with this change. This buffer should not exist, and > > even 100 packets is a stretch. Any module hitting this needs to recondiser > > how it deals with flow control > > Brad Beckmann wrote: > I would agree with you that the buffer should not exist, but removing the > buffer goes well beyond this patch. The worst part about the buffer is the > size is not visible to the sender. It is a really bad design, but for now we > need to get around the immediate problem. Later we can discuss how and who > will fix this right. > > We've discussed the proposal in the past to increase the size, but > unfortunately we have not came to a resolution. We absolutely need to > resolve this now. We cannot use the ruby tester with our upcoming GPU model > without this change. > > The important thing to keep in mind is that the RubyTester is designed to > stress the protocol logic and it creates contention scenarios that would not > exist in a real system. The RubyTester finds bugs in the matter of seconds > that may not be encountered in hours of real workload simulation. It does > this by sending large bursts of racey requests. Bottomline: the RubyTester > needs this large value. > > Andreas Hansson wrote: > I would argue that if you need flow control, you should not use the > QueuedPorts, and rather use a "bare-metal" MasterPort, and deal with the flow > control. There is no point in adding flow control (or buffer visibility) to > the QueuePort in my opinion. > > I'd suggest to switch to a MasterPort either as part of this patch, or a > patch before it. That way you have no implicit buffer, and no need to create > kilobytes of invisible buffering in the system.
The RubyPort and the Ruby Tester predate MasterPorts and QueuedPorts. Your suggestion goes far beyond this patch. Reimplementing RubyPort's m5 ports to inherit from a different base port is a very signficant change with many, many implications beyond the Ruby Tester. That is a pretty unreasonable request. Please keep in mind that I was not the one who decided RubyPort to use QueuedPorts. That decision was made back in 2012 with patches from your group 8914:8c3bd7bea667, and 8922:17f037ad8918. - Brad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2776/#review6152 ----------------------------------------------------------- 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
