[ofa-general] [PATCH] opensm: multi lid routing balancing for updn/minhop

[Al Chu]

On Sun, 2008-03-09 at 15:41 +0000, Sasha Khapyorsky wrote:
> On 11:01 Tue 04 Mar     , Al Chu wrote:
> > From 391e2e3cdd08b205bdb94203660c0aacb8212d5a Mon Sep 17 00:00:00 2001
> > From: Albert L. Chu <chu11 at llnl.gov>
> > Date: Mon, 3 Mar 2008 10:39:43 -0800
> > Subject: [PATCH] support balanced multi-lid routing
> > 
> > 
> > Signed-off-by: Albert L. Chu <chu11 at llnl.gov>
> 
> Applied. Thanks.
> 
> I just have a few stylistic comments below. I applied the patch as is
> with hope that we can fix it later in subsequent patches (fixed some
> already).

Thanks.  I'll keep the below in mind for the future.

> An error code (ie 'ERR XXXX') should be unique, so it is better to not
> change this.

My bad on this one.  I assumed the error codes were sequential, so I
just picked the next one (the previous error codes were 3A09 and 3A0A).
I'll do a grep next time :-)

Al

> [snip...]
> 
> > diff --git a/opensm/include/opensm/osm_switch.h b/opensm/include/opensm/osm_switch.h
> > index e2fe86d..dbb2552 100644
> > --- a/opensm/include/opensm/osm_switch.h
> > +++ b/opensm/include/opensm/osm_switch.h
> > @@ -158,6 +158,33 @@ typedef struct _osm_switch {
> >  *	Switch object
> >  *********/
> >  
> > +/****s* OpenSM: Switch/osm_switch_guid_count_t
> > +* NAME
> > +*       osm_switch_guid_count_t
> > +*
> > +* DESCRIPTION
> > +*       Stores system and node guids and the number of
> 
> Tab character instead of spaces.
> 
> [snip...]
> 
> > --- a/opensm/opensm/osm_switch.c
> > +++ b/opensm/opensm/osm_switch.c
> > @@ -219,16 +219,99 @@ osm_switch_get_fwd_tbl_block(IN const osm_switch_t * const p_sw,
> >  
> >  /**********************************************************************
> >   **********************************************************************/
> > +static osm_switch_guid_count_t *
> > +osm_switch_find_guid_common(IN const osm_switch_t * const p_sw,
> > +			    IN osm_switch_guid_count_t * remote_guids,
> > +			    IN uint16_t * p_num_remote_guids,
> > +			    IN uint8_t port_num,
> > +			    IN int find_sys_guid,
> > +			    IN int find_node_guid)
> 
> We are using osm_ prefixes only for public functions, for statics names
> can be shorter.
> 
> [snip...]
> 
> > diff --git a/opensm/opensm/osm_ucast_mgr.c b/opensm/opensm/osm_ucast_mgr.c
> > index 1aa5ea9..d7fc4d3 100644
> > --- a/opensm/opensm/osm_ucast_mgr.c
> > +++ b/opensm/opensm/osm_ucast_mgr.c
> > @@ -209,31 +209,21 @@ __osm_ucast_mgr_process_port(IN osm_ucast_mgr_t * const p_mgr,
> >  	   in providing better routing in LMC > 0 situations
> >  	 */
> >  	uint16_t lids_per_port = 1 << p_mgr->p_subn->opt.lmc;
> > -	uint64_t *remote_sys_guids = NULL;
> > -	uint64_t *remote_node_guids = NULL;
> > -	uint16_t num_used_sys = 0;
> > -	uint16_t num_used_nodes = 0;
> > +	osm_switch_guid_count_t *remote_guids = NULL;
> > +	uint16_t num_used_guids = 0;
> > +	osm_switch_guid_count_t *p_remote_guid_used = NULL;
> >  
> >  	OSM_LOG_ENTER(p_mgr->p_log);
> >  
> >  	if (lids_per_port > 1) {
> > -		remote_sys_guids = malloc(sizeof(uint64_t) * lids_per_port);
> > -		if (remote_sys_guids == NULL) {
> > -			OSM_LOG(p_mgr->p_log, OSM_LOG_ERROR, "ERR 3A09: "
> > +		remote_guids = malloc(sizeof(osm_switch_guid_count_t) * lids_per_port);
> > +		if (remote_guids == NULL) {
> > +			osm_log(p_mgr->p_log, OSM_LOG_ERROR,
> > +				"__osm_ucast_mgr_process_port: ERR 3A0B: "
> 
> An error code (ie 'ERR XXXX') should be unique, so it is better to not
> change this.
> 
> Sasha
-- 
Albert Chu
chu11 at llnl.gov
925-422-5311
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory