> On June 23, 2015, 9:19 p.m., Nilay Vaish wrote: > > There are several style issues. The writer should read > > gem5.org/Coding_Style. > > I am still trying to understand all the synchronization code. So it will > > take > > me sometime before I am able to review that code.
Nilay, I've addressed your feedback in the lastest patch. Thank you for your diligent review. I've dropped the issues about the rate and the representation as double which I've explained in the comments. Curtis > On June 23, 2015, 9:19 p.m., Nilay Vaish wrote: > > src/dev/multi_etherlink.cc, line 207 > > <http://reviews.gem5.org/r/2826/diff/3/?file=45211#file45211line207> > > > > If the checkpoint patch from Andreas Sandberg goes in first, you would > > need to fix the serialize functions. Yes. We have code internally that makes these adjustments and we'll be sure to apply it if that is the eventual patch ordering. > On June 23, 2015, 9:19 p.m., Nilay Vaish wrote: > > src/dev/multi_etherlink.hh, line 128 > > <http://reviews.gem5.org/r/2826/diff/3/?file=45210#file45210line128> > > > > Please don't call it rate. In abstraction ticks per byte is also a > > rate, but I think the general meaning for rate is something amount of work > > carried out per unit time. You probably want to work with the bandwidth of > > the link. The live range of this variable is just two lines, but nonetheless I changed it to "invBW" since it is essentially an inverse bandwidth, time over data rather than data over time. > On June 23, 2015, 9:19 p.m., Nilay Vaish wrote: > > src/dev/multi_etherlink.hh, line 112 > > <http://reviews.gem5.org/r/2826/diff/3/?file=45210#file45210line112> > > > > double? The simulator does not understand fractional ticks. We understand the concern, but we just multiple this number by a packet's size to compute the number of ticks it will take to send. Since we round that number up to the next tick, we don't see any problem here. The double allows us to nicely handle arbitrary link bandwidths. > On June 23, 2015, 9:19 p.m., Nilay Vaish wrote: > > src/dev/multi_etherlink.cc, line 192 > > <http://reviews.gem5.org/r/2826/diff/3/?file=45211#file45211line192> > > > > I think there is some confusion between what the python parameter > > represents and what the C++ code expects for this ticksPerByte variable. The definition of NetworkBandwidth in src/python/m5/params.py explains this oddness. It starts in the standard bits-per-second unit scheme, but in the getValue() method it gets flipped upside down to ticks per byte. So in fact this oddness dates back to 2007 when Nate Binkert introduced these definitions. Since what you typically want to do (which is what this code does) is multiply the number of bytes you have by some constant to get the transmission time in ticks, it works rather nicely. - Curtis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2826/#review6528 ----------------------------------------------------------- On June 24, 2015, 11:56 p.m., Curtis Dunham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2826/ > ----------------------------------------------------------- > > (Updated June 24, 2015, 11:56 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Multi gem5 is an extension to gem5 to enable parallel simulation of a > distributed system (e.g. simulation of a pool of machines > connected by Ethernet links). A multi gem5 run consists of seperate gem5 > processes running in parallel (potentially on different hosts/slots on > a cluster). Each gem5 process executes the simulation of a component of the > simulated distributed system (e.g. a multi-core board with an Ethernet NIC). > > The patch implements the "distributed" Ethernet link device > (dev/src/multi_etherlink.[hh.cc]). This device will send/receive > (simulated) Ethernet packets to/from peer gem5 processes. The interface > to talk to the peer gem5 processes is defined in dev/src/multi_iface.hh and > in tcp_iface.hh. > > There is also a central message server process (util/multi/tcp_server.[hh,cc]) > which acts like an Ethernet switch and transfers messages among the gem5 > peers. > > A multi gem5 simulations can be kicked off by the util/multi/gem5-multi.sh > wrapper script. > > Checkpoint support will follow in a subsequent patch. > > > Diffs > ----- > > src/dev/Ethernet.py d02b45a554b52c68cce41e1b3895fb8582a639dd > src/dev/SConscript d02b45a554b52c68cce41e1b3895fb8582a639dd > src/dev/etherpkt.hh d02b45a554b52c68cce41e1b3895fb8582a639dd > src/dev/etherpkt.cc d02b45a554b52c68cce41e1b3895fb8582a639dd > src/dev/multi_etherlink.hh PRE-CREATION > src/dev/multi_etherlink.cc PRE-CREATION > src/dev/multi_iface.hh PRE-CREATION > src/dev/multi_iface.cc PRE-CREATION > src/dev/multi_packet.hh PRE-CREATION > src/dev/multi_packet.cc PRE-CREATION > src/dev/tcp_iface.hh PRE-CREATION > src/dev/tcp_iface.cc PRE-CREATION > util/multi/Makefile PRE-CREATION > util/multi/bootscript.rcS PRE-CREATION > util/multi/gem5-multi.sh PRE-CREATION > util/multi/tcp_server.hh PRE-CREATION > util/multi/tcp_server.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/2826/diff/ > > > Testing > ------- > > > Thanks, > > Curtis Dunham > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
