Hi Nilay,

On Tue, Apr 14, 2015 at 11:33 AM, Nilay Vaish <[email protected]> wrote:

> On Tue, 14 Apr 2015, Joel Hestness wrote:
>
>  This patch didn't address my requested change (
>> http://reviews.gem5.org/r/2702/), and shouldn't have been checked in. The
>> AbstractController should not have or even need another pointer to the
>> RubySystem, because it can access it with g_system_ptr. This is a dead
>> simple fix. Adding the pointer is going in the wrong direction for
>> abstraction.
>>
>>
> Joel, in my opinion we should be eliminating global / static variables. I
> have personally found them to be a problem while trying to parallelize Ruby.
>

So, I have a couple issues with this:
 1) You didn't raise this opinion in the review itself, but instead made
the decision without seeking consensus.
 2) Duplicating a global pointer as members within other objects does not
change the way that the pointers need to be handled when parallelizing
Ruby. If there is a shared data member within the RubySystem that needs to
be locked to ensure consistency, you'll need lock that variable no matter
how other objects access the RubySystem. Further, the RubySystem global
pointer is set exactly once in the RubySystem constructor and never allowed
to be changed again. Therefore, there should be no worries about races on
the global pointer itself, AND distributing RubySystem pointers into other
objects is more likely to generate potential races if there is ever a need
to update those pointers. Thus, I strongly disagree with your opinion on
distributing pointers into the AbstractControllers.

  Am I missing something here?

  Joel


-- 
  Joel Hestness
  PhD Candidate, Computer Architecture
  Dept. of Computer Science, University of Wisconsin - Madison
  http://pages.cs.wisc.edu/~hestness/
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to