----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3595/#review8713 -----------------------------------------------------------
I like the idea of starting the sync on a pseudo op very much. I cannot see the usefulness of stopping the sync after it started though. Do you have a use case in mind? I have a few comments below (mainly minor nits). However, I think there is also a basic issue with the CPU suspend approach. A CPU can wake up whenever there is an interrupt or even a snoop request. That should be taken care of somehow I assume. src/dev/net/dist_iface.hh (line 461) <http://reviews.gem5.org/r/3595/#comment7529> The type should be bool. Minor nit: the name could be changed to something like "syncStartOnPseudoOp" to reflect the meaning better. src/dev/net/dist_iface.hh (line 627) <http://reviews.gem5.org/r/3595/#comment7530> Minor nit: could you rename this method to "toggleSync()" or similar? That would better describe the functionality. The word "switch" ususally refers to the network switch model in the dist-gem5 context so it is a bit confusing here. src/dev/net/dist_iface.cc (line 71) <http://reviews.gem5.org/r/3595/#comment7531> This change effectively disables the use of the start_tick parameter. It is because 'nextAt' is overwritten in SyncEvent::start() by the max tick of all participating gem5 - which will be 0 at the first global barrier (if we do not start the sync on pseudo op). However, I think the pseudo op is a much better way to defer the sync start. Could you just remove the 'start_tick' option altogether? src/dev/net/dist_iface.cc (line 830) <http://reviews.gem5.org/r/3595/#comment7532> Cannot another active thread hit another stop-sync pesudo op while this thread is still suspended? Maybe all active threads should be suspended here? - Gabor Dozsa On Aug. 4, 2016, 4:51 p.m., Michael LeBeane wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3595/ > ----------------------------------------------------------- > > (Updated Aug. 4, 2016, 4:51 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11561:1414a40eb1e2 > --------------------------- > dev: Add m5 op to toggle synchronization for dist-gem5. > This patch adds the ability for an application to request dist-gem5 to begin/ > end synchronization using an m5 op. When toggling on sync, all nodes agree on > the next sync point based on the maximum of all nodes' ticks. CPUs are > suspended until the sync point to avoid sending network messages until sync > has > been enabled. Toggling off sync acts like a global execution barrier, where > all CPUs are disabled until every node reaches the toggle off point. This > avoids tricky situations such as one node hitting a toggle off followed by a > toggle on before the other nodes hit the first toggle off. > > > Diffs > ----- > > configs/common/Options.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/arch/x86/isa/decoder/two_byte_opcodes.isa > 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/Ethernet.py 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_etherlink.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/dist_packet.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/dev/net/tcp_iface.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.hh 91f58918a76abf1a1dedcaa70a9b95789da7b88c > src/sim/pseudo_inst.cc 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5op_x86.S 91f58918a76abf1a1dedcaa70a9b95789da7b88c > util/m5/m5ops.h 91f58918a76abf1a1dedcaa70a9b95789da7b88c > > Diff: http://reviews.gem5.org/r/3595/diff/ > > > Testing > ------- > > > Thanks, > > Michael LeBeane > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
