[ofa-general] Re: {PATCH] [2] opensm: per mesh data

[Sasha Khapyorsky]

On 01:26 Tue 11 Nov     , Robert Pearson wrote:
> Sasha,
> 
> Here is the second patch implementing the mesh analysis algorithm.
> 
> This patch:
>       - creates a data structure, mesh_t, that holds per mesh information
>       - adds a pointer to this structure in lash_t
>       - creates methods to allocate and free memory for mesh_t
>       - adds osm_ prefix to global routine names (oops)
>       - calls create and cleanup methods
> 
> 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 1467440..8313614 100644
> --- a/opensm/include/opensm/osm_mesh.h
> +++ b/opensm/include/opensm/osm_mesh.h
> @@ -41,6 +41,18 @@
>  
>  struct _lash;
>  
> -int do_mesh_analysis(struct _lash *p_lash);
> +/*
> + * per fabric mesh info
> + */
> +typedef struct _mesh {
> +	int num_class;			/* number of switch classes */
> +	int *class_type;		/* index of first switch found for
> each class */
> +	int *class_count;		/* population of each class */
> +	int dimension;			/* mesh dimension */
> +	int *size;			/* an array to hold size of mesh */
> +} mesh_t;
> +
> +void osm_mesh_cleanup(struct _lash *p_lash);
> +int osm_do_mesh_analysis(struct _lash *p_lash);
>  
>  #endif
> diff --git a/opensm/include/opensm/osm_ucast_lash.h
> b/opensm/include/opensm/osm_ucast_lash.h
> index 646e9a3..1ae3bb6 100644
> --- a/opensm/include/opensm/osm_ucast_lash.h
> +++ b/opensm/include/opensm/osm_ucast_lash.h
> @@ -95,6 +95,7 @@ typedef struct _lash {
>  	cdg_vertex_t ****cdg_vertex_matrix;
>  	int *num_mst_in_lane;
>  	int ***virtual_location;
> +	mesh_t *mesh;
>  } lash_t;
>  
>  #endif
> diff --git a/opensm/opensm/osm_mesh.c b/opensm/opensm/osm_mesh.c
> index 7943274..c97925b 100644
> --- a/opensm/opensm/osm_mesh.c
> +++ b/opensm/opensm/osm_mesh.c
> @@ -41,6 +41,7 @@
>  #endif				/* HAVE_CONFIG_H */
>  
>  #include <stdio.h>
> +#include <stdlib.h>
>  #include <opensm/osm_switch.h>
>  #include <opensm/osm_opensm.h>
>  #include <opensm/osm_log.h>
> @@ -48,15 +49,72 @@
>  #include <opensm/osm_ucast_lash.h>
>  
>  /*
> + * osm_mesh_cleanup - free per mesh resources
> + */
> +void osm_mesh_cleanup(lash_t *p_lash)
> +{
> +	mesh_t *mesh = p_lash->mesh;
> +
> +	if (mesh) {
> +		if (mesh->class_type)
> +			free(mesh->class_type);
> +
> +		if (mesh->class_count)
> +			free(mesh->class_count);
> +
> +		free(mesh);
> +
> +		p_lash->mesh = NULL;
> +	}
> +}
> +
> +/*
> + * mesh_create - allocate per mesh resources
> + */
> +static int mesh_create(lash_t *p_lash)
> +{
> +	osm_log_t *p_log = &p_lash->p_osm->log;
> +	mesh_t *mesh;
> +
> +	if(!(mesh = p_lash->mesh = calloc(1, sizeof(mesh_t)))) {
> +		OSM_LOG(p_log, OSM_LOG_ERROR, "Failed allocating mesh - out
> of memory\n");
> +		return -1;
> +	}
> +
> +	if (!(mesh->class_type = calloc(p_lash->num_switches, sizeof(int))))
> {
> +		OSM_LOG(p_log, OSM_LOG_ERROR, "Failed allocating
> mesh->class_type - out of memory\n");
> +		free(mesh);
> +		return -1;
> +	}
> +
> +	if (!(mesh->class_count = calloc(p_lash->num_switches,
> sizeof(int)))) {
> +		OSM_LOG(p_log, OSM_LOG_ERROR, "Failed allocating
> mesh->class_count - out of memory\n");
> +		free(mesh->class_type);
> +		free(mesh);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
>   * do_mesh_analysis
>   */
> -int do_mesh_analysis(lash_t *p_lash)
> +int osm_do_mesh_analysis(lash_t *p_lash)
>  {
>  	int ret = 0;
>  	osm_log_t *p_log = &p_lash->p_osm->log;
>  
>  	OSM_LOG_ENTER(p_log);
>  
> +	/*
> +	 * allocate per mesh data structures
> +	 */
> +	if (mesh_create(p_lash)) {
> +		OSM_LOG_EXIT(p_log);
> +		return -1;
> +	}
> +
>  	printf("lash: do_mesh_analysis stub called\n");
>  
>  	OSM_LOG_EXIT(p_log);
> diff --git a/opensm/opensm/osm_ucast_lash.c b/opensm/opensm/osm_ucast_lash.c
> index e10371c..3577cca 100644
> --- a/opensm/opensm/osm_ucast_lash.c
> +++ b/opensm/opensm/osm_ucast_lash.c
> @@ -825,7 +825,7 @@ static int lash_core(lash_t * p_lash)
>  
>  	OSM_LOG_ENTER(p_log);
>  
> -	if (p_lash->p_osm->subn.opt.do_mesh_analysis &&
> do_mesh_analysis(p_lash)) {
> +	if (p_lash->p_osm->subn.opt.do_mesh_analysis &&
> osm_do_mesh_analysis(p_lash)) {
>  		OSM_LOG(p_log, OSM_LOG_ERROR, "Mesh analysis failed\n");
>  		goto Exit;
>  	}
> @@ -1124,6 +1124,8 @@ static void lash_cleanup(lash_t * p_lash)
>  		free(p_lash->switches);
>  	}
>  	p_lash->switches = NULL;
> +
> +	osm_mesh_cleanup(p_lash);
>  }

lash_cleanup() is called at start of LASH processor, so mesh will keep
allocated data between routing calculation cycles. But as far as I can
see it is not used there. Also osm_mesh_cleanup() is not called on lash
deletion and we have a memory leak.

Maybe osm_mesh_cleanup() should be static function (mesh_cleanup()) and
be called somewhere at end of osm_do_mesh_analysis()?

Sasha