ruby: Fix Topology throttle connections

Information
Submitter:	Joel Hestness
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 9803:749372538449
---------------------------
ruby: Fix Topology throttle connections

The Topology source sets up input and output buffers for each of the external
nodes of a topology by indexing on Ruby's generated controller unique IDs.
These unique IDs are found by adding the MachineType_base_number to the version
number of each controller (see any generated *_Controller.cc - init() calls
getToNetQueue and getFromNetQueue using m_version + base). However, the
Topology object used the cntrl_id - which is required to be unique across all
controllers - to index the controllers list as they are being connected to
their input and output buffers. If the cntrl_ids did not match the Ruby unique
ID, the throttles end up connected to incorrectly indexed nodes in the network,
resulting in packets traversing incorrect network paths. This patch fixes the
Topology indexing scheme by using the Ruby unique ID to match that of the
SimpleNetwork buffer vectors.

Testing Done

Ran all Ruby regressions and observed no stats changes. It should be noted that
more complicated SimpleNetwork topologies connecting numerous components will
very likely witness throttle connection changes, which can cause changes in
network performance, especially with throttle bandwidth_factor of 16B/cycle.

It should also be noted that this patch removes the last hard dependency on the
use of cntrl_ids in all Ruby code. Based on the potential confusion between
Ruby's generated unique IDs and cntrl_ids, I recommend that cntrl_ids eventually
be eliminated from the codebase.

Joel, I am of the view that we should eliminate ruby's internal
numbering instead. The cntrl_id is something that appears in
the config.ini file and hence is visible to the user. But ruby's
internal numbering of controllers never gets exposed.

Joel Hestness July 2, 2013, 7:13 p.m. (July 2, 2013, 7:13 p.m.)

I'm a fan of Ruby's unique numbering for a few reasons:

1) Because Ruby controllers are instantiated and initialized in 2 separate phases, it is guaranteed that the Ruby unique IDs are setup before any initialization, since they only depend on the number of controllers, not their orders, types or otherwise. This is really simple compared to instantiation ordering or another complicated scheme in the protocol config files.

2) All of the code for setting up the machine type base number for the Ruby unique IDs is rolled into SLICC generated code, so the user need not have any exposure to it. It would be nice to just rely on this; basically every time that I've setup or modified a protocol configuration file (4 or 5 experiences), getting the cntrl_ids correct has been a pain and error prone. Without the change I'm proposing here, if the cntrl_ids are not sequential from 0-(# cntrls - 1) (which is easy to mess up), you get a MessageBuffer connection fatal with any interconnect using Topology.cc. This fatal would never be tripped if using Ruby unique IDs, based on simple-to-setup controller version numbers.

3) The cntrl_ids are unnecessary as far as I can tell, and they don't inform the user about anything that the Ruby unique IDs or version number cannot. Probably the most (only?) important place where we must be able to identify a particular controller is in a protocol trace. These traces already use the version number, which is printed in the config.ini and is user controlled. Effectively, cntrl_ids are a duplicate of Ruby unique IDs with a user defined machine type base number. I'm not aware of a need for this flexibility.

Anyway, this is a bit of an aside from this review request: Do you want to make a decision on using cntrl_ids vs. Ruby unique IDs before this patch goes through? It does fix a known bug, and I feel it does so in an appropriate direction.

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Status: Closed (submitted)