Hi Sasha,
Sasha Khapyorsky wrote:
Hi Yevgeny,
On 18:03 Wed 16 May , Yevgeny Kliteynik wrote:
Hi Hal,
This patch optimizes fabric ranking similar to the fat-tree ranking.
All the root switches are marked with rank and added to the BFS list,
and only then ranking of rest of the fabric begins.
Please apply to master.
Signed-off-by: Yevgeny Kliteynik <[EMAIL PROTECTED]>
---
Basically looks good. However couple comments below.
opensm/opensm/osm_ucast_updn.c | 66
+++++++++++++++++----------------------
1 files changed, 29 insertions(+), 37 deletions(-)
diff --git a/opensm/opensm/osm_ucast_updn.c b/opensm/opensm/osm_ucast_updn.c
index 5cebd9b..9574216 100644
--- a/opensm/opensm/osm_ucast_updn.c
+++ b/opensm/opensm/osm_ucast_updn.c
@@ -408,53 +408,49 @@ Exit :
/* rank is a SWITCH for BFS purpose */
static int
updn_subn_rank(
- IN uint64_t root_guid,
- IN uint8_t base_rank,
+ IN uint32_t num_guids,
'num_guids' should not be fixed-size integer just compiler friendly
'unsigned' is fine.
NP.
+ IN uint64_t* guid_list,
IN updn_t* p_updn )
{
osm_switch_t *p_sw;
- uint32_t rank = base_rank;
osm_physp_t *p_physp, *p_remote_physp;
cl_qlist_t list;
cl_status_t did_cause_update;
struct updn_node *u, *remote_u;
uint8_t num_ports, port_num;
osm_log_t *p_log = &p_updn->p_osm->log;
+ uint32_t idx = 0;
Ditto.
NP
OSM_LOG_ENTER( p_log, updn_subn_rank );
+ cl_qlist_init(&list);
- p_sw = osm_get_switch_by_guid(&p_updn->p_osm->subn,
cl_hton64(root_guid));
- if(!p_sw)
- {
- osm_log( p_log, OSM_LOG_ERROR,
- "updn_subn_rank: ERR AA05: "
- "Root switch GUID 0x%" PRIx64 " not found\n", root_guid );
- OSM_LOG_EXIT( p_log );
- return 1;
- }
-
- osm_log( p_log, OSM_LOG_VERBOSE,
- "updn_subn_rank: "
- "Ranking starts from GUID 0x%" PRIx64 "\n", root_guid );
-
- u = p_sw->priv;
- u->is_root = 1;
+ /* Rank all the roots and add them to list */
- /* Rank the first guid chosen anyway since it's the base rank */
- osm_log( p_log, OSM_LOG_DEBUG,
- "updn_subn_rank: "
- "Ranking port GUID 0x%" PRIx64 "\n", root_guid );
+ for (idx = 0; idx < num_guids; idx++)
+ {
+ /* Apply the ranking for each guid given by user - bypass illegal ones
*/
+ p_sw = osm_get_switch_by_guid(&p_updn->p_osm->subn,
cl_hton64(guid_list[idx]));
+ if(!p_sw)
+ {
+ osm_log( p_log, OSM_LOG_ERROR,
+ "updn_subn_rank: ERR AA05: "
+ "Root switch GUID 0x%" PRIx64 " not found\n",
guid_list[idx] );
+ continue;
+ }
- __updn_update_rank(u, rank);
+ u = p_sw->priv;
+ u->is_root = 1;
Now when root switches are always ranked first 'is_root' field is not
needed anymore, (!u->rank) answers this.
Agree
- cl_qlist_init(&list);
- cl_qlist_insert_tail(&list, &u->list);
+ osm_log( p_log, OSM_LOG_DEBUG,
+ "updn_subn_rank: "
+ "Ranking root port GUID 0x%" PRIx64 "\n", guid_list[idx] );
+ __updn_update_rank(u, 0);
+ cl_qlist_insert_tail(&list, &u->list);
+ }
/* BFS the list till it's empty */
while (!cl_is_qlist_empty(&list))
{
- rank++;
-
u = (struct updn_node *)cl_qlist_remove_head(&list);
/* Go over all remote nodes and rank them (if not already visited) */
p_sw = u->sw;
@@ -483,7 +479,7 @@ updn_subn_rank(
{
remote_u = p_remote_physp->p_node->sw->priv;
port_guid = p_remote_physp->port_guid;
- did_cause_update = __updn_update_rank(remote_u, rank);
+ did_cause_update = __updn_update_rank(remote_u, u->rank+1);
osm_log( p_log, OSM_LOG_DEBUG,
"updn_subn_rank: "
@@ -500,8 +496,8 @@ updn_subn_rank(
/* Print Summary of ranking */
osm_log( p_log, OSM_LOG_VERBOSE,
"updn_subn_rank: "
- "Rank Info :\n\t Root Guid = 0x%" PRIx64 "\n\t Max Node Rank =
%d\n",
- root_guid, rank );
+ "Subnet ranking completed. Max Node Rank = %d\n",
+ remote_u->rank );
'remote_u' can be not initialized here. Another issue is that it can be
initialized but to remote switch which has lower than max rank (when
did_cause_update = 0).
Right, good catch.
I'll issue a new patch.
Thanks.
-- Yevgeny
The rest is fine.
Sasha
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
