Review Board 2.0.15


dist: Distributed Ethernet link support for distributed gem5 simulations

Review Request #3228 - Created Nov. 19, 2015 and submitted

Information
Curtis Dunham
gem5
default
Reviewers
Default 
Distributed gem5 is the result of the convergence effort between multi-gem5 and pd-gem5 (from Univ. of Wisconsin). It relies on the base multi-gem5 infrastructure for packet forwarding, synchronisation and checkpointing but combines those with the elaborated network switch model from pd-gem5.

   

  


 


 

  

  






 
 
Issue Summary

 

  

   
    
    5
   
   
    
    0
   
   
    
    5
   
   
    
    0
   
  

  

   

    
    
   

   

    
    
   

  

 

 

  

   
    

     Description
     From
     Last Updated
     Status
    

   
   







    















    



    



    



    





















   
  

 









  
 


  

 


















 













 



 


















 













 



 













 



 

  


 

  

   

   
Review request changed

   
Updated  (Jan. 7, 2016, 2:45 p.m.)

  

  


   

    Status:
    

    Closed (submitted)

    
   







   


   




  

 


  

 

















 



 

  


 


  



   

   
Steve Reinhardt

   
Posted  (Jan. 7, 2016, 3:14 p.m.)


  


  


  

   
Why was this patch committed?  There are no ship-its, and there was no warning.  I was complaining about minor things to buy myself a little time, and also because I hoped the changes between the similar files would be easier to detect/review if rename was used.  I saw the updates on reviewboard and made a mental note that now I need to go back and re-review, and then next thing I know I see the changeset messages going by...



  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    Hi Steve,
    
I committed it because of this comment you made on this RB: "Given that it's clearly an improvement over the status quo (since it includes input from pd-gem5) I don't have a problem with committing first and addressing any issues I run across later, once the style issues are addressed."
    

    Our documentation says (http://gem5.org/Submitting_Contributions):
    
"If your patch has been on reviewboard for a while without getting any reviews (or re-revires after you've posted changes), please email the gem5-dev list. If you have commit access, it is fair to give warning via email that you intend to commit the changes at some future date (e.g., a week out from the date of the email) if you do not hear any objections. Please do not simply commit a patch without giving warning on the gem5-dev list."
    

    I did precisely this; I made a plea for further review on the list on December 10, stating a desire to commit on December 18, and this was the only feedback I received, and to reiterate, the feedback we did get didn't sound much like an objection.
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    It's not a big deal; my comment about being OK with submitting first and fixing later does still apply.  It just seemed very odd that you posted an update to reviewboard and left it up for all of 45 minutes before you committed.  Why even bother to post the update if you're not going to give people time to look at it?
    

    I'm familiar with the rule you quote.  I know it's not clearly stated there, but to me, getting a comment that's not an explicit "ship it" is sufficient to reset the process.  Also once you've posted an update you're no longer in the situation that you've gone "a while" without any "re-reviews after posting changes".  I'm not saying you clearly broke the rule, just explaining how my interpretation makes it look that way to me :).
    

    Ironically if you had just committed the code on the 18th before I had time to look at it, I would not have any grounds for complaint...
    

    Again, not a big deal, just a misunderstanding.  In general though I would request that people be conservative about assuming things are ready to commit if patches have been recently updated and/or there are outstanding comments w/o explicit ship-its.
    
    
    
   
    3.