> 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?
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? - Michael ----------------------------------------------------------- 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
