[ofa-general] Re: [PATCH v3 1/2] opensm: setup routing engine when in use and delete when fail

[Sasha Khapyorsky]

Hi Eli,

On 15:37 Thu 19 Mar     , Eli Dorfman (Voltaire) wrote:
> setup routing engine when in use and delete when fail
> 
> setup routing engine and allocate resources before use.
> delete resources when routing algorithm fails.
> this will save allocation for routing algorithms that are not used.
> 
> Signed-off-by: Eli Dorfman <elid at voltaire.com>
> ---
>  opensm/include/opensm/osm_opensm.h |    6 ++++++
>  opensm/opensm/osm_opensm.c         |   10 ++--------
>  opensm/opensm/osm_ucast_mgr.c      |   17 +++++++++++++++++
>  3 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/opensm/include/opensm/osm_opensm.h b/opensm/include/opensm/osm_opensm.h
> index c121be4..8d1b276 100644
> --- a/opensm/include/opensm/osm_opensm.h
> +++ b/opensm/include/opensm/osm_opensm.h
> @@ -109,6 +109,9 @@ typedef enum _osm_routing_engine_type {
>  } osm_routing_engine_type_t;
>  /***********/
>  
> +struct osm_routing_engine;
> +struct osm_opensm;
> +
>  /****s* OpenSM: OpenSM/osm_routing_engine
>  * NAME
>  *	struct osm_routing_engine
> @@ -122,6 +125,7 @@ typedef enum _osm_routing_engine_type {
>  struct osm_routing_engine {
>  	const char *name;
>  	void *context;
> +	int (*setup) (struct osm_routing_engine *re, struct osm_opensm *p_osm);
>  	int (*build_lid_matrices) (void *context);
>  	int (*ucast_build_fwd_tables) (void *context);
>  	void (*ucast_dump_tables) (void *context);
> @@ -523,5 +527,7 @@ extern volatile unsigned int osm_exit_flag;
>  *  Set to one to cause all threads to leave
>  *********/
>  
> +void osm_update_routing_engines(osm_opensm_t *osm, const char *engine_names);
> +

This function is not implemented in this patch. Please move it to
related patch.

>  END_C_DECLS
>  #endif				/* _OSM_OPENSM_H_ */
> diff --git a/opensm/opensm/osm_opensm.c b/opensm/opensm/osm_opensm.c
> index 50d1349..9739122 100644
> --- a/opensm/opensm/osm_opensm.c
> +++ b/opensm/opensm/osm_opensm.c
> @@ -169,14 +169,7 @@ static void setup_routing_engine(osm_opensm_t *osm, const char *name)
>  			memset(re, 0, sizeof(struct osm_routing_engine));
>  
>  			re->name = m->name;
> -			if (m->setup(re, osm)) {
> -				OSM_LOG(&osm->log, OSM_LOG_VERBOSE,
> -					"setup of routing"
> -					" engine \'%s\' failed\n", name);
> -				return;
> -			}
> -			OSM_LOG(&osm->log, OSM_LOG_DEBUG,
> -				"\'%s\' routing engine set up\n", re->name);
> +			re->setup = m->setup;

Ok, only 'setup' callback is initialized here. That is fine. But later
in destroy_routing_engines() for all routing engines delete() method is
called unconditionally, which obviously should crash OpenSM.

It is still be broken IMO.

Sasha