Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-10-13 Thread Gabor Dozsa 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/#review8830
---

Ship it!


Ship it! Thanks for the fixes!

- Gabor Dozsa


On Oct. 12, 2016, 3:51 p.m., Michael LeBeane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3595/
> ---
> 
> (Updated Oct. 12, 2016, 3: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
> -
> 
>   util/m5/m5op_x86.S e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   util/m5/m5ops.h e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   util/m5/m5op.h e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/dist_iface.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/dist_etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/arch/x86/isa/decoder/two_byte_opcodes.isa 
> e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/Ethernet.py e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   configs/common/Options.py e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/sim/pseudo_inst.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/sim/pseudo_inst.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/tcp_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/tcp_iface.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/dist_packet.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
>   src/dev/net/dist_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
> 
> Diff: http://reviews.gem5.org/r/3595/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael LeBeane
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-10-12 Thread Michael LeBeane 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/
---

(Updated Oct. 12, 2016, 3: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 (updated)
-

  util/m5/m5op_x86.S e92bf392bf4302e2e88e19907e7fc981f59e777d 
  util/m5/m5ops.h e92bf392bf4302e2e88e19907e7fc981f59e777d 
  util/m5/m5op.h e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/dev/net/dist_iface.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/dev/net/dist_etherlink.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/arch/x86/isa/decoder/two_byte_opcodes.isa 
e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/dev/net/Ethernet.py e92bf392bf4302e2e88e19907e7fc981f59e777d 
  configs/common/Options.py e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/sim/pseudo_inst.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/sim/pseudo_inst.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/dev/net/tcp_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/dev/net/tcp_iface.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/dev/net/dist_packet.hh e92bf392bf4302e2e88e19907e7fc981f59e777d 
  src/dev/net/dist_iface.cc e92bf392bf4302e2e88e19907e7fc981f59e777d 

Diff: http://reviews.gem5.org/r/3595/diff/


Testing
---


Thanks,

Michael LeBeane

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-10-12 Thread Michael LeBeane 


> On Sept. 9, 2016, 2:53 p.m., Gabor Dozsa wrote:
> > src/dev/net/dist_iface.cc, line 71
> > 
> >
> > 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?

I did a quick test and the start_tick approach still appears to work with this 
patch.

nextAt is assigned the value of start_tick for all the nodes and the switch in 
Synch::init().  SyncSwitch::progress() only updates nextAt(which determines the 
max tick of all paticipating processes) if it is less than send_tick.  At time 
zero this is not true, so the first global barrier will be scheduled at 
start_tick.


- Michael


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/#review8713
---


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-10-06 Thread Gabor Dozsa 


> On Sept. 9, 2016, 2:53 p.m., Gabor Dozsa wrote:
> > 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.
> 
> Michael LeBeane wrote:
> The use case for switching off would be if you have multiple regions of 
> interest in the code and no need to sync in between.  You could also just run 
> the simulation a bunch of times and move the start sync point, but I think 
> it’s much cleaner to run the app once and toggle the sync.
> 
> I suppose a spurious wakeup is possible; I had not really thought about 
> it.  I haven't seen CPUs wakeup in practice, but I don't really have any 
> devices that would generate an interrupt/snoop when all the cores are asleep.
> 
> One alternative way to do this would be to not suspend any of the cores 
> and modify the sync to support all the devices being on different ticks when 
> sync starts.  We would need to remember what the start time was for all the 
> participating nodes and use that time to enforce synchronization instead of 
> assuming everyone is on the same tick.  I avoided doing this since suspending 
> the CPUs until we reach the max of all nodes ticks' was easier.
> 
> Michael LeBeane wrote:
> Before I address your specific review comments, I think it would be good 
> to resolve the high-level concern about suspending CPUs.  Do you have any 
> thoughts about the alternative approach I suggested?  I'm not a huge fan of 
> it since having all the nodes on different ticks makes correlating traces 
> between nodes very difficult.  Did you have a better approach in mind?

I agree that the alternative approach would make it difficult to correlate 
events between nodes. I would suggest to go ahead with your current CPU 
suspend/wakup approach for now. Would it be possible to issue a warning in case 
of a spurious wakeup event? If something goes wrong the warning could give a 
hint.


- Gabor


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/#review8713
---


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-10-03 Thread Michael LeBeane 


> On Sept. 9, 2016, 2:53 p.m., Gabor Dozsa wrote:
> > 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.
> 
> Michael LeBeane wrote:
> The use case for switching off would be if you have multiple regions of 
> interest in the code and no need to sync in between.  You could also just run 
> the simulation a bunch of times and move the start sync point, but I think 
> it’s much cleaner to run the app once and toggle the sync.
> 
> I suppose a spurious wakeup is possible; I had not really thought about 
> it.  I haven't seen CPUs wakeup in practice, but I don't really have any 
> devices that would generate an interrupt/snoop when all the cores are asleep.
> 
> One alternative way to do this would be to not suspend any of the cores 
> and modify the sync to support all the devices being on different ticks when 
> sync starts.  We would need to remember what the start time was for all the 
> participating nodes and use that time to enforce synchronization instead of 
> assuming everyone is on the same tick.  I avoided doing this since suspending 
> the CPUs until we reach the max of all nodes ticks' was easier.

Before I address your specific review comments, I think it would be good to 
resolve the high-level concern about suspending CPUs.  Do you have any thoughts 
about the alternative approach I suggested?  I'm not a huge fan of it since 
having all the nodes on different ticks makes correlating traces between nodes 
very difficult.  Did you have a better approach in mind?


- Michael


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/#review8713
---


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-09-13 Thread Michael LeBeane 


> On Sept. 9, 2016, 2:53 p.m., Gabor Dozsa wrote:
> > 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.

The use case for switching off would be if you have multiple regions of 
interest in the code and no need to sync in between.  You could also just run 
the simulation a bunch of times and move the start sync point, but I think it’s 
much cleaner to run the app once and toggle the sync.

I suppose a spurious wakeup is possible; I had not really thought about it.  I 
haven't seen CPUs wakeup in practice, but I don't really have any devices that 
would generate an interrupt/snoop when all the cores are asleep.

One alternative way to do this would be to not suspend any of the cores and 
modify the sync to support all the devices being on different ticks when sync 
starts.  We would need to remember what the start time was for all the 
participating nodes and use that time to enforce synchronization instead of 
assuming everyone is on the same tick.  I avoided doing this since suspending 
the CPUs until we reach the max of all nodes ticks' was easier.


- Michael


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/#review8713
---


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-09-13 Thread Michael LeBeane 


> On Aug. 30, 2016, 3:39 p.m., Michael LeBeane wrote:
> > Any comments on this patch?
> 
> Mohammad Alian wrote:
> I just have some high level question before goring through the code:
> 
> Can you explain the use-case for this patch? Probably it means to speed 
> up the simulation but why do we need to switch on/off synchronization at some 
> points?
> 
> When you suspend CPUs, then how your simulation will gaurantee forward 
> progress? Doesn't this lead to dead-lock in some situations?
> 
> Thanks.

The use case is, like you inferred, to speed up simulation when you don't need 
to synchronize.  There is currently a method to support this using ticks, but I 
prefer to use source code annotations.

The CPUs suspend until the largest tick of all participating CPUs is reached.  
This is to ensure that we really have started synchronizing after executing the 
pseudo-op, else we could start sending network messages before we are ready.  I 
don't see a deadlock situation here, can you elaborate on your concern?


- Michael


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/#review8693
---


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-09-09 Thread Gabor Dozsa 

---
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)


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)


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)


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)


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-09-02 Thread Mohammad Alian 


> On Aug. 30, 2016, 3:39 p.m., Michael LeBeane wrote:
> > Any comments on this patch?

I just have some high level question before goring through the code:

Can you explain the use-case for this patch? Probably it means to speed up the 
simulation but why do we need to switch on/off synchronization at some points?

When you suspend CPUs, then how your simulation will gaurantee forward 
progress? Doesn't this lead to dead-lock in some situations?

Thanks.


- Mohammad


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/#review8693
---


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3595: dev: Add m5 op to toggle synchronization for dist-gem5.

2016-08-30 Thread Michael LeBeane 

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3595/#review8693
---


Any comments on this patch?

- Michael LeBeane


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
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev