[ofa-general] Re: [PATCH][3] opensm: per mesh node information

[Sasha Khapyorsky]

On 02:06 Tue 11 Nov     , Robert Pearson wrote:
> Sasha,
> 
> This is the third patch implementing the mesh analysis algorithm
> 
> This patch
>       - creates per mesh node (e.g. switch) data structure mesh_node_t
>       - adds a pointer to mesh_node_t in the switch_t structure
>       - implements create and cleanup methods for node_t
>       - calls these in switch_create and swich_delete in *lash.c
> 
> Regards,
> 
> Bob Pearson
> 
> Signed-off-by: Bob Pearson <rpearson at systemfabricworks.com>
> ----
> diff --git a/opensm/include/opensm/osm_mesh.h
> b/opensm/include/opensm/osm_mesh.h
> index 8313614..78af086 100644
> --- a/opensm/include/opensm/osm_mesh.h
> +++ b/opensm/include/opensm/osm_mesh.h
> @@ -40,6 +40,39 @@
>  #define OSM_UCAST_MESH_H
>  
>  struct _lash;
> +struct _switch;
> +
> +enum mesh_node_type {
> +	mesh_type_none,
> +	mesh_type_cartesian,
> +};
> +
> +/*
> + * per switch to switch link info
> + */
> +typedef struct _link {
> +	int switch_id;
> +	int link_id;
> +	int *ports;
> +	int num_ports;
> +	int next_port;
> +} link_t;
> +
> +/*
> + * per switch node mesh info
> + */
> +typedef struct _mesh_node {
> +	unsigned int num_links;		/* number of 'links' to adjacent
> switches */
> +	link_t **links;			/* per link information */
> +	int *axes;			/* used to hold and reorder assigned
> axes */
> +	int *coord;			/* mesh coordinates of switch */
> +	int **matrix;			/* distances between adjacent
> switches */
> +	int *poly;			/* characteristic polynomial of
> matrix */
> +					/* used as an invariant
> classification */
> +	enum mesh_node_type type;
> +	int dimension;			/* apparent dimension of mesh around
> node */
> +	int temp;			/* temporary holder for distance
> info */
> +} mesh_node_t;
>  
>  /*
>   * per fabric mesh info
> @@ -55,4 +88,7 @@ typedef struct _mesh {
>  void osm_mesh_cleanup(struct _lash *p_lash);
>  int osm_do_mesh_analysis(struct _lash *p_lash);
>  
> +void osm_mesh_node_cleanup(struct _switch *sw);
> +int osm_mesh_node_create(struct _lash *p_lash, struct _switch *sw);
> +
>  #endif
> diff --git a/opensm/include/opensm/osm_ucast_lash.h
> b/opensm/include/opensm/osm_ucast_lash.h
> index 1ae3bb6..c037571 100644
> --- a/opensm/include/opensm/osm_ucast_lash.h
> +++ b/opensm/include/opensm/osm_ucast_lash.h
> @@ -81,6 +81,7 @@ typedef struct _switch {
>  		unsigned out_link;
>  		unsigned lane;
>  	} *routing_table;
> +	mesh_node_t *node;
>  	unsigned int num_connections;
>  	int *virtual_physical_port_table;
>  	int *phys_connections;
> diff --git a/opensm/opensm/osm_mesh.c b/opensm/opensm/osm_mesh.c
> index c97925b..6ef397c 100644
> --- a/opensm/opensm/osm_mesh.c
> +++ b/opensm/opensm/osm_mesh.c
> @@ -98,7 +98,7 @@ static int mesh_create(lash_t *p_lash)
>  }
>  
>  /*
> - * do_mesh_analysis
> + * osm_do_mesh_analysis
>   */
>  int osm_do_mesh_analysis(lash_t *p_lash)
>  {
> @@ -121,3 +121,83 @@ int osm_do_mesh_analysis(lash_t *p_lash)
>  
>  	return ret;
>  }
> +
> +/*
> + * osm_mesh_node_cleanup - cleanup per switch resources
> + */
> +void osm_mesh_node_cleanup(switch_t *sw)
> +{
> +	int i;
> +	mesh_node_t *node = sw->node;
> +	unsigned num_ports = sw->p_sw->num_ports;
> +
> +	if (node) {
> +		if (node->links) {
> +			for (i = 0; i < num_ports; i++) {
> +				if (node->links[i]) {
> +					if (node->links[i]->ports)
> +						free(node->links[i]->ports);
> +					free(node->links[i]);
> +				}
> +			}
> +			free(node->links);
> +		}
> +
> +		if (node->poly)
> +			free(node->poly);
> +
> +		if (node->matrix) {
> +			for (i = 0; i < node->num_links; i++) {
> +				if (node->matrix[i])
> +					free(node->matrix[i]);
> +			}
> +			free(node->matrix);
> +		}
> +
> +		if (node->axes)
> +			free(node->axes);
> +
> +		free(node);
> +
> +		sw->node = NULL;
> +	}
> +}
> +
> +/*
> + * osm_mesh_node_create - allocate per switch resources
> + */
> +int osm_mesh_node_create(lash_t *p_lash, switch_t *sw)
> +{
> +	osm_log_t *p_log = &p_lash->p_osm->log;
> +	int i;
> +	mesh_node_t *node;
> +	unsigned num_ports = sw->p_sw->num_ports;
> +
> +	if (!(node = sw->node = calloc(1, sizeof(mesh_node_t)))) {
> +		OSM_LOG(p_log, OSM_LOG_ERROR, "Failed allocating mesh node -
> out of memory\n");
> +		return -1;
> +	}
> +
> +	if (!(node->links = calloc(num_ports, sizeof(link_t *))))
> +		goto err;
> +
> +	for (i = 0; i < num_ports; i++) {
> +		if (!(node->links[i] = calloc(1, sizeof(link_t))) ||
> +		    !(node->links[i]->ports = calloc(num_ports,
> sizeof(int))))
> +			goto err;
> +	}

Assuming that ports array is preallocated, wouldn't it be simpler to
define link as:

typedef struct _link {
	int switch_id;
	int link_id;
	int num_ports;
	int next_port;
	int ports[0];
} link_t;

, and then:

	node->links[i] = calloc(1, sizeof(link_t *) + num_ports * sizeof(int))))

?

(Similar optimizations are probably relevant in other places).

Sasha

> +
> +	if (!(node->axes = calloc(num_ports, sizeof(int))))
> +		goto err;
> +
> +	for (i = 0; i < num_ports; i++) {
> +		node->links[i]->switch_id = NONE;
> +	}
> +
> +	return 0;
> +
> +err:
> +	OSM_LOG(p_log, OSM_LOG_ERROR, "Failed allocating mesh node - out of
> memory\n");
> +	osm_mesh_node_cleanup(sw);
> +	return -1;
> +}
> diff --git a/opensm/opensm/osm_ucast_lash.c b/opensm/opensm/osm_ucast_lash.c
> index 3577cca..b9394af 100644
> --- a/opensm/opensm/osm_ucast_lash.c
> +++ b/opensm/opensm/osm_ucast_lash.c
> @@ -651,6 +651,9 @@ static switch_t *switch_create(lash_t * p_lash, unsigned
> id, osm_switch_t * p_sw
>  		sw->phys_connections[i] = NONE;
>  	}
>  
> +	if (osm_mesh_node_create(p_lash, sw))
> +		return -1;
> +
>  	sw->p_sw = p_sw;
>  	if (p_sw)
>  		p_sw->priv = sw;
> @@ -660,6 +663,8 @@ static switch_t *switch_create(lash_t * p_lash, unsigned
> id, osm_switch_t * p_sw
>  
>  static void switch_delete(switch_t * sw)
>  {
> +	osm_mesh_node_cleanup(sw);
> +
>  	if (sw->dij_channels)
>  		free(sw->dij_channels);
>  	if (sw->virtual_physical_port_table)
> 
>