> On Sept. 1, 2016, 3:33 p.m., Jason Lowe-Power wrote: > > What testing did you perform to make sure all of the protocols were > > modified correctly? > > > > Most of these changes seem reasonable to me, but I know from experience > > that even when the SLICC changes seem like they are right, if they aren't > > tested carefully there's almost always bugs. > > Michael LeBeane wrote: > The sequencer changes have been tested pretty thoroughly in a custom > protocol; the public SLICC files only with the regression tester. I'm not > sure how much coverage that provides for DMA other than checking if it > compiles. > > Jason Lowe-Power wrote: > I'm not sure what to do about this. Maybe others in the community will > speak up ;). > > I don't feel comfortable pushing a patch with code that we know hasn't > been tested at all. Specifically with Ruby/SLICC, this has bitten me before. > I've updated gem5 and all of sudden my simluations are broken because someone > changed a protocol without testing it. IMO, code needs to be tested at least > somewhat before it's pushed into the mainline. > > However, I also understand that there isn't any testing infrastructure > for most of the protocols, and we can't ask you to solve that problem before > pushing the patch in. > > Do others have thoughts on this (reoccurring) problem? > > For this specific patch, can you run your workload that use DMA with > protocols other than your internal protocol? > > Michael LeBeane wrote: > It would take some effort, but I do have some DMA intensive networking > benchmarks that should be able to run over the public protocols. I wouldn't > be able to share these or add them to the regression tester (they are > proprietary). > > I know that doesn't help at all with the more general problem of poor > tester coverage, but would that be an acceptable solution for this patch? > > Jason Lowe-Power wrote: > I would appreciate it if you ran those tests. It doesn't bother me nearly > as much that they are proprietary tests. At least we'll know that something > executed correctly with the other protocols! Improving the coverage of the > regression tests is hugely outside of the scope for this patch. > > I'd still like to hear what others think about this problem in the longer > term.
Another potential issue: The queue of requests that has been created, does it officially need to be visible to any functional access? In other words, has it "happened" yet? For the queued ports we have functions to query the queued packets on a functional access, but that does not seem to be present in the patch. - Andreas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3591/#review8698 ----------------------------------------------------------- On Aug. 30, 2016, 3:54 p.m., Michael LeBeane wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3591/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2016, 3:54 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11557:5d6fcf14c77e > --------------------------- > ruby: Allow multiple outstanding DMA requests > DMA sequencers and protocols can currently only issue one DMA access at a > time. > This patch implements the necessary functionality to support multiple > outstanding DMA requests in Ruby. > > > Diffs > ----- > > src/mem/ruby/system/Sequencer.py 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/ruby/system/DMASequencer.hh > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/ruby/system/DMASequencer.cc > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_hammer-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/RubySlicc_Types.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_CMP_directory-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MOESI_CMP_token-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MESI_Two_Level-dma.sm > 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > src/mem/protocol/MI_example-dma.sm 985d9b9a68bf20e22ba65f7398dde0193e6ca076 > > Diff: http://reviews.gem5.org/r/3591/diff/ > > > Testing > ------- > > > Thanks, > > Michael LeBeane > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
