On Tue, 14 Apr 2015, Joel Hestness wrote:
Hi Nilay,
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.
The patch itself is not at variance with what I might have done to address
the issue. Since no one else currently commits patches meant for ruby, I
tend to decide on my own what should be done. Along with this patch, I
committed three other patches that were not discussed at all. Again, in
all such cases, I do what I feel should be done.
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?
You can use g_system_ptr, or you can make a local copy of the pointer.
To me both ways are equally fine, neither seems better. It is just that I
prefer not using global variables, so I probably would have gone the same
way as the patch does.
--
Nilay
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
