[ofa-general] Re: [PATCH RFC] opensm: mcast mgr improvements

Sasha Khapyorsky sashak at voltaire.com
Mon Dec 31 07:48:15 PST 2007


On 17:35 Mon 31 Dec     , Yevgeny Kliteynik wrote:
>  Hal Rosenstock wrote:
> > On Sun, 2007-12-30 at 18:16 +0000, Sasha Khapyorsky wrote:
> >> On 08:38 Sun 30 Dec     , Hal Rosenstock wrote:
> >>> On Sat, 2007-12-29 at 18:27 +0000, Sasha Khapyorsky wrote:
> >>>> This improves handling of mcast join/leave requests storming. Now mcast
> >>>> routing will be recalculated for all mcast groups where changes occurred
> >>>> and not one by one. For this it queues mcast groups instead of mcast
> >>>> rerouting requests, this also makes state_mgr idle queue obsolete.
> >>> Looks like a nice improvement.
> >>>
> >>> What testing has been done with this change ? Can you comment on any
> >>> results ?
> >> osmtest, basic ipoib, SA db and MFTs dump diffs. Didn't find any
> >> problem.
> > What size topologies ? real and/or simulated ?
> >>> For which branches is this change being proposed ?
> >> I think it should go to OFED 1.3.
> > Perhaps if there is sufficient soak time on real life topologies and
> > other torture tests for this.
> 
>  I will include this patch in the nightly simulation today,

Thanks!

Sasha

>  but currently I don't have access to any real cluster.
> 
>  -- Yevgeny
> 
> 
> > -- Hal
> >> Sasha
> >>
> >>> -- Hal
> >>>
> >>>> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> >>>> ---
> >>>>
> >>>> Hi Yevgeny,
> >>>>
> >>>> For me it looks that it should solve the original problem (mcast group
> >>>> list is purged in osm_mcast_mgr_process()). Could you review and ideally
> >>>> test it? Thanks.
> >>>>
> >>>> Sasha
> >>>>
> >>>> ---
> >>>>  opensm/include/opensm/osm_mcast_mgr.h |   14 +--
> >>>>  opensm/include/opensm/osm_multicast.h |    2 +
> >>>>  opensm/include/opensm/osm_sm.h        |    2 +
> >>>>  opensm/include/opensm/osm_state_mgr.h |   95 -----------------
> >>>>  opensm/opensm/osm_mcast_mgr.c         |  187 
> >>>> +++++++++++++++------------------
> >>>>  opensm/opensm/osm_sm.c                |   70 ++++++-------
> >>>>  opensm/opensm/osm_state_mgr.c         |  138 +------------------------
> >>>>  7 files changed, 130 insertions(+), 378 deletions(-)
> >>>>
> >>>> diff --git a/opensm/include/opensm/osm_mcast_mgr.h 
> >>>> b/opensm/include/opensm/osm_mcast_mgr.h
> >>>> index 3e0b761..47b67ed 100644
> >>>> --- a/opensm/include/opensm/osm_mcast_mgr.h
> >>>> +++ b/opensm/include/opensm/osm_mcast_mgr.h
> >>>> @@ -100,7 +100,6 @@ typedef struct _osm_mcast_mgr {
> >>>>  	osm_req_t *p_req;
> >>>>  	osm_log_t *p_log;
> >>>>  	cl_plock_t *p_lock;
> >>>> -
> >>>>  } osm_mcast_mgr_t;
> >>>>  /*
> >>>>  * FIELDS
> >>>> @@ -253,25 +252,22 @@ osm_signal_t osm_mcast_mgr_process(IN 
> >>>> osm_mcast_mgr_t * const p_mgr);
> >>>>  *	Multicast Manager, Node Info Response Controller
> >>>>  *********/
> >>>>  -/****f* OpenSM: Multicast Manager/osm_mcast_mgr_process_mgrp_cb
> >>>> +/****f* OpenSM: Multicast Manager/osm_mcast_mgr_process_mgroups
> >>>>  * NAME
> >>>> -*	osm_mcast_mgr_process_mgrp_cb
> >>>> +*	osm_mcast_mgr_process_mgroups
> >>>>  *
> >>>>  * DESCRIPTION
> >>>> -*	Callback entry point for the osm_mcast_mgr_process_mgrp function.
> >>>> +*	Process only requested mcast groups.
> >>>>  *
> >>>>  * SYNOPSIS
> >>>>  */
> >>>>  osm_signal_t
> >>>> -osm_mcast_mgr_process_mgrp_cb(IN void *const Context1, IN void *const 
> >>>> Context2);
> >>>> +osm_mcast_mgr_process_mgroups(IN osm_mcast_mgr_t *p_mgr);
> >>>>  /*
> >>>>  * PARAMETERS
> >>>> -*	(Context1) p_mgr
> >>>> +*	p_mgr
> >>>>  *		[in] Pointer to an osm_mcast_mgr_t object.
> >>>>  *
> >>>> -*	(Context2) p_mgrp
> >>>> -*		[in] Pointer to the multicast group to process.
> >>>> -*
> >>>>  * RETURN VALUES
> >>>>  *	IB_SUCCESS
> >>>>  *
> >>>> diff --git a/opensm/include/opensm/osm_multicast.h 
> >>>> b/opensm/include/opensm/osm_multicast.h
> >>>> index 729a2ea..f442a45 100644
> >>>> --- a/opensm/include/opensm/osm_multicast.h
> >>>> +++ b/opensm/include/opensm/osm_multicast.h
> >>>> @@ -50,6 +50,7 @@
> >>>>   #include <iba/ib_types.h>
> >>>>  #include <complib/cl_qmap.h>
> >>>> +#include <complib/cl_qlist.h>
> >>>>  #include <complib/cl_spinlock.h>
> >>>>  #include <opensm/osm_base.h>
> >>>>  #include <opensm/osm_mtree.h>
> >>>> @@ -121,6 +122,7 @@ const char *osm_get_mcast_req_type_str(IN 
> >>>> osm_mcast_req_type_t req_type);
> >>>>  * SYNOPSIS
> >>>>  */
> >>>>  typedef struct osm_mcast_mgr_ctxt {
> >>>> +	cl_list_item_t list_item;
> >>>>  	ib_net16_t mlid;
> >>>>  	osm_mcast_req_type_t req_type;
> >>>>  	ib_net64_t port_guid;
> >>>> diff --git a/opensm/include/opensm/osm_sm.h 
> >>>> b/opensm/include/opensm/osm_sm.h
> >>>> index 4c6ce27..a676cd6 100644
> >>>> --- a/opensm/include/opensm/osm_sm.h
> >>>> +++ b/opensm/include/opensm/osm_sm.h
> >>>> @@ -140,6 +140,8 @@ typedef struct osm_sm {
> >>>>  	cl_dispatcher_t *p_disp;
> >>>>  	cl_plock_t *p_lock;
> >>>>  	atomic32_t sm_trans_id;
> >>>> +	cl_spinlock_t mgrp_lock;
> >>>> +	cl_qlist_t mgrp_list;
> >>>>  	osm_req_t req;
> >>>>  	osm_resp_t resp;
> >>>>  	osm_ni_rcv_t ni_rcv;
> >>>> diff --git a/opensm/include/opensm/osm_state_mgr.h 
> >>>> b/opensm/include/opensm/osm_state_mgr.h
> >>>> index dada097..f51593a 100644
> >>>> --- a/opensm/include/opensm/osm_state_mgr.h
> >>>> +++ b/opensm/include/opensm/osm_state_mgr.h
> >>>> @@ -109,8 +109,6 @@ typedef struct _osm_state_mgr {
> >>>>  	osm_stats_t *p_stats;
> >>>>  	struct _osm_sm_state_mgr *p_sm_state_mgr;
> >>>>  	const osm_sm_mad_ctrl_t *p_mad_ctrl;
> >>>> -	cl_spinlock_t idle_lock;
> >>>> -	cl_qlist_t idle_time_list;
> >>>>  	cl_plock_t *p_lock;
> >>>>  	cl_event_t *p_subnet_up_event;
> >>>>  	osm_sm_state_t state;
> >>>> @@ -172,99 +170,6 @@ typedef struct _osm_state_mgr {
> >>>>  *	State Manager object
> >>>>  *********/
> >>>>  -/****s* OpenSM: State Manager/_osm_idle_item
> >>>> -* NAME
> >>>> -*	_osm_idle_item
> >>>> -*
> >>>> -* DESCRIPTION
> >>>> -*	Idle item.
> >>>> -*
> >>>> -* SYNOPSIS
> >>>> -*/
> >>>> -
> >>>> -typedef osm_signal_t(*osm_pfn_start_t) (IN void *context1, IN void 
> >>>> *context2);
> >>>> -
> >>>> -typedef void
> >>>> - (*osm_pfn_done_t) (IN void *context1, IN void *context2);
> >>>> -
> >>>> -typedef struct _osm_idle_item {
> >>>> -	cl_list_item_t list_item;
> >>>> -	void *context1;
> >>>> -	void *context2;
> >>>> -	osm_pfn_start_t pfn_start;
> >>>> -	osm_pfn_done_t pfn_done;
> >>>> -} osm_idle_item_t;
> >>>> -
> >>>> -/*
> >>>> -* FIELDS
> >>>> -*	list_item
> >>>> -*		list item.
> >>>> -*
> >>>> -*	context1
> >>>> -*		Context pointer
> >>>> -*
> >>>> -*	context2
> >>>> -*		Context pointer
> >>>> -*
> >>>> -*	pfn_start
> >>>> -*		Pointer to the start function.
> >>>> -*
> >>>> -*	pfn_done
> >>>> -*		Pointer to the dine function.
> >>>> -* SEE ALSO
> >>>> -*	State Manager object
> >>>> -*********/
> >>>> -
> >>>> -/****f* OpenSM: State Manager/osm_state_mgr_process_idle
> >>>> -* NAME
> >>>> -*	osm_state_mgr_process_idle
> >>>> -*
> >>>> -* DESCRIPTION
> >>>> -*	Formulates the osm_idle_item and inserts it into the queue and
> >>>> -*	signals the state manager.
> >>>> -*
> >>>> -* SYNOPSIS
> >>>> -*/
> >>>> -
> >>>> -ib_api_status_t
> >>>> -osm_state_mgr_process_idle(IN osm_state_mgr_t * const p_mgr,
> >>>> -			   IN osm_pfn_start_t pfn_start,
> >>>> -			   IN osm_pfn_done_t pfn_done,
> >>>> -			   void *context1, void *context2);
> >>>> -
> >>>> -/*
> >>>> -* PARAMETERS
> >>>> -*	p_mgr
> >>>> -*		[in] Pointer to a State Manager object to construct.
> >>>> -*
> >>>> -*	pfn_start
> >>>> -*		[in] Pointer the start function which will be called at
> >>>> -*			idle time.
> >>>> -*
> >>>> -*	pfn_done
> >>>> -*		[in] pointer the done function which will be called
> >>>> -*			when outstanding smps is zero
> >>>> -*
> >>>> -*	context1
> >>>> -*		[in] Pointer to void
> >>>> -*
> >>>> -*	context2
> >>>> -*		[in] Pointer to void
> >>>> -*
> >>>> -* RETURN VALUE
> >>>> -*	IB_SUCCESS or IB_ERROR
> >>>> -*
> >>>> -* NOTES
> >>>> -*	Allows osm_state_mgr_destroy
> >>>> -*
> >>>> -*	Calling osm_state_mgr_construct is a prerequisite to calling any 
> >>>> other
> >>>> -*	method except osm_state_mgr_init.
> >>>> -*
> >>>> -* SEE ALSO
> >>>> -*	State Manager object, osm_state_mgr_init,
> >>>> -*	osm_state_mgr_destroy
> >>>> -*********/
> >>>> -
> >>>>  /****f* OpenSM: State Manager/osm_state_mgr_construct
> >>>>  * NAME
> >>>>  *	osm_state_mgr_construct
> >>>> diff --git a/opensm/opensm/osm_mcast_mgr.c 
> >>>> b/opensm/opensm/osm_mcast_mgr.c
> >>>> index 50b95fd..f51a45a 100644
> >>>> --- a/opensm/opensm/osm_mcast_mgr.c
> >>>> +++ b/opensm/opensm/osm_mcast_mgr.c
> >>>> @@ -815,7 +815,7 @@ static osm_mtree_node_t 
> >>>> *__osm_mcast_mgr_branch(osm_mcast_mgr_t * const p_mgr,
> >>>>  	}
> >>>>   	free(list_array);
> >>>> -      Exit:
> >>>> +Exit:
> >>>>  	OSM_LOG_EXIT(p_mgr->p_log);
> >>>>  	return (p_mtn);
> >>>>  }
> >>>> @@ -932,7 +932,7 @@ __osm_mcast_mgr_build_spanning_tree(osm_mcast_mgr_t 
> >>>> * const p_mgr,
> >>>>  		"Configured MLID 0x%X for %u ports, max tree depth = %u\n",
> >>>>  		cl_ntoh16(osm_mgrp_get_mlid(p_mgrp)), count, max_depth);
> >>>>  -      Exit:
> >>>> +Exit:
> >>>>  	OSM_LOG_EXIT(p_mgr->p_log);
> >>>>  	return (status);
> >>>>  }
> >>>> @@ -1171,7 +1171,7 @@ osm_mcast_mgr_process_single(IN osm_mcast_mgr_t * 
> >>>> const p_mgr,
> >>>>  		}
> >>>>  	}
> >>>>  -      Exit:
> >>>> +Exit:
> >>>>  	OSM_LOG_EXIT(p_mgr->p_log);
> >>>>  	return (status);
> >>>>  }
> >>>> @@ -1254,63 +1254,55 @@ osm_mcast_mgr_process_tree(IN osm_mcast_mgr_t * 
> >>>> const p_mgr,
> >>>>  							   port_guid);
> >>>>  	}
> >>>>  -      Exit:
> >>>> +Exit:
> >>>>  	OSM_LOG_EXIT(p_mgr->p_log);
> >>>>  	return (status);
> >>>>  }
> >>>>   
> >>>> /**********************************************************************
> >>>>   Process the entire group.
> >>>> -
> >>>>   NOTE : The lock should be held externally!
> >>>>   
> >>>> **********************************************************************/
> >>>> -static osm_signal_t
> >>>> -osm_mcast_mgr_process_mgrp(IN osm_mcast_mgr_t * const p_mgr,
> >>>> -			   IN osm_mgrp_t * const p_mgrp,
> >>>> -			   IN osm_mcast_req_type_t req_type,
> >>>> -			   IN ib_net64_t port_guid)
> >>>> +static ib_api_status_t
> >>>> +mcast_mgr_process_mgrp(IN osm_mcast_mgr_t * const p_mgr,
> >>>> +		       IN osm_mgrp_t * const p_mgrp,
> >>>> +		       IN osm_mcast_req_type_t req_type,
> >>>> +		       IN ib_net64_t port_guid)
> >>>>  {
> >>>> -	osm_signal_t signal = OSM_SIGNAL_DONE;
> >>>>  	ib_api_status_t status;
> >>>> -	osm_switch_t *p_sw;
> >>>> -	cl_qmap_t *p_sw_tbl;
> >>>> -	boolean_t pending_transactions = FALSE;
> >>>>   	OSM_LOG_ENTER(p_mgr->p_log, osm_mcast_mgr_process_mgrp);
> >>>>  -	p_sw_tbl = &p_mgr->p_subn->sw_guid_tbl;
> >>>> -
> >>>>  	status = osm_mcast_mgr_process_tree(p_mgr, p_mgrp, req_type, 
> >>>> port_guid);
> >>>>  	if (status != IB_SUCCESS) {
> >>>>  		osm_log(p_mgr->p_log, OSM_LOG_ERROR,
> >>>> -			"osm_mcast_mgr_process_mgrp: ERR 0A19: "
> >>>> +			"mcast_mgr_process_mgrp: ERR 0A19: "
> >>>>  			"Unable to create spanning tree (%s)\n",
> >>>>  			ib_get_err_str(status));
> >>>> -
> >>>>  		goto Exit;
> >>>>  	}
> >>>> +	p_mgrp->last_tree_id = p_mgrp->last_change_id;
> >>>>  -	/*
> >>>> -	   Walk the switches and download the tables for each.
> >>>> +	/* Remove MGRP only if osm_mcm_port_t count is 0 and
> >>>> +	 * Not a well known group
> >>>>  	 */
> >>>> -	p_sw = (osm_switch_t *) cl_qmap_head(p_sw_tbl);
> >>>> -	while (p_sw != (osm_switch_t *) cl_qmap_end(p_sw_tbl)) {
> >>>> -		signal = __osm_mcast_mgr_set_tbl(p_mgr, p_sw);
> >>>> -		if (signal == OSM_SIGNAL_DONE_PENDING)
> >>>> -			pending_transactions = TRUE;
> >>>> -
> >>>> -		p_sw = (osm_switch_t *) cl_qmap_next(&p_sw->map_item);
> >>>> +	if (cl_qmap_count(&p_mgrp->mcm_port_tbl) == 0 && !p_mgrp->well_known) 
> >>>> {
> >>>> +		osm_log(p_mgr->p_log, OSM_LOG_DEBUG,
> >>>> +			"mcast_mgr_process_mgrp: "
> >>>> +			"Destroying mgrp with lid:0x%X\n",
> >>>> +			cl_ntoh16(p_mgrp->mlid));
> >>>> +		/* Send a Report to any InformInfo registered for
> >>>> +		   Trap 67 : MCGroup delete */
> >>>> +		osm_mgrp_send_delete_notice(p_mgr->p_subn, p_mgr->p_log,
> >>>> +					    p_mgrp);
> >>>> +		cl_qmap_remove_item(&p_mgr->p_subn->mgrp_mlid_tbl,
> >>>> +				    (cl_map_item_t *) p_mgrp);
> >>>> +		osm_mgrp_delete(p_mgrp);
> >>>>  	}
> >>>>  -	osm_dump_mcast_routes(p_mgr->p_subn->p_osm);
> >>>> -
> >>>> -      Exit:
> >>>> +Exit:
> >>>>  	OSM_LOG_EXIT(p_mgr->p_log);
> >>>> -
> >>>> -	if (pending_transactions == TRUE)
> >>>> -		return (OSM_SIGNAL_DONE_PENDING);
> >>>> -	else
> >>>> -		return (OSM_SIGNAL_DONE);
> >>>> +	return status;
> >>>>  }
> >>>>   
> >>>> /**********************************************************************
> >>>> @@ -1321,14 +1313,13 @@ osm_signal_t osm_mcast_mgr_process(IN 
> >>>> osm_mcast_mgr_t * const p_mgr)
> >>>>  	osm_switch_t *p_sw;
> >>>>  	cl_qmap_t *p_sw_tbl;
> >>>>  	cl_qmap_t *p_mcast_tbl;
> >>>> +	cl_qlist_t *p_list = &p_mgr->p_subn->p_osm->sm.mgrp_list;
> >>>>  	osm_mgrp_t *p_mgrp;
> >>>> -	ib_api_status_t status;
> >>>>  	boolean_t pending_transactions = FALSE;
> >>>>   	OSM_LOG_ENTER(p_mgr->p_log, osm_mcast_mgr_process);
> >>>>   	p_sw_tbl = &p_mgr->p_subn->sw_guid_tbl;
> >>>> -
> >>>>  	p_mcast_tbl = &p_mgr->p_subn->mgrp_mlid_tbl;
> >>>>  	/*
> >>>>  	   While holding the lock, iterate over all the established
> >>>> @@ -1343,16 +1334,8 @@ osm_signal_t osm_mcast_mgr_process(IN 
> >>>> osm_mcast_mgr_t * const p_mgr)
> >>>>  		/* We reached here due to some change that caused a heavy sweep
> >>>>  		   of the subnet. Not due to a specific multicast request.
> >>>>  		   So the request type is subnet_change and the port guid is 0. */
> >>>> -		status = osm_mcast_mgr_process_tree(p_mgr, p_mgrp,
> >>>> -						    OSM_MCAST_REQ_TYPE_SUBNET_CHANGE,
> >>>> -						    0);
> >>>> -		if (status != IB_SUCCESS) {
> >>>> -			osm_log(p_mgr->p_log, OSM_LOG_ERROR,
> >>>> -				"osm_mcast_mgr_process: ERR 0A20: "
> >>>> -				"Unable to create spanning tree (%s)\n",
> >>>> -				ib_get_err_str(status));
> >>>> -		}
> >>>> -
> >>>> +		mcast_mgr_process_mgrp(p_mgr, p_mgrp,
> >>>> +				       OSM_MCAST_REQ_TYPE_SUBNET_CHANGE, 0);
> >>>>  		p_mgrp = (osm_mgrp_t *) cl_qmap_next(&p_mgrp->map_item);
> >>>>  	}
> >>>>  @@ -1364,10 +1347,14 @@ osm_signal_t osm_mcast_mgr_process(IN 
> >>>> osm_mcast_mgr_t * const p_mgr)
> >>>>  		signal = __osm_mcast_mgr_set_tbl(p_mgr, p_sw);
> >>>>  		if (signal == OSM_SIGNAL_DONE_PENDING)
> >>>>  			pending_transactions = TRUE;
> >>>> -
> >>>>  		p_sw = (osm_switch_t *) cl_qmap_next(&p_sw->map_item);
> >>>>  	}
> >>>>  +	while (!cl_is_qlist_empty(p_list)) {
> >>>> +		cl_list_item_t *p = cl_qlist_remove_head(p_list);
> >>>> +		free(p);
> >>>> +	}
> >>>> +
> >>>>  	CL_PLOCK_RELEASE(p_mgr->p_lock);
> >>>>   	OSM_LOG_EXIT(p_mgr->p_log);
> >>>> @@ -1395,79 +1382,79 @@ osm_mgrp_t *__get_mgrp_by_mlid(IN 
> >>>> osm_mcast_mgr_t * const p_mgr,
> >>>>   
> >>>> /**********************************************************************
> >>>>    This is the function that is invoked during idle time to handle the
> >>>> -  process request. Context1 is simply the osm_mcast_mgr_t*, Context2
> >>>> -  hold the mlid, port guid and action (join/leave/delete) required.
> >>>> +  process request for mcast groups where join/leave/delete was 
> >>>> required.
> >>>>   
> >>>> **********************************************************************/
> >>>> -osm_signal_t
> >>>> -osm_mcast_mgr_process_mgrp_cb(IN void *const Context1, IN void *const 
> >>>> Context2)
> >>>> +osm_signal_t osm_mcast_mgr_process_mgroups(osm_mcast_mgr_t * p_mgr)
> >>>>  {
> >>>> -	osm_mcast_mgr_t *p_mgr = (osm_mcast_mgr_t *) Context1;
> >>>> +	cl_qlist_t *p_list = &p_mgr->p_subn->p_osm->sm.mgrp_list;
> >>>> +	osm_switch_t *p_sw;
> >>>> +	cl_qmap_t *p_sw_tbl;
> >>>>  	osm_mgrp_t *p_mgrp;
> >>>>  	ib_net16_t mlid;
> >>>> -	osm_signal_t signal = OSM_SIGNAL_DONE;
> >>>> -	osm_mcast_mgr_ctxt_t *p_ctxt = (osm_mcast_mgr_ctxt_t *) Context2;
> >>>> -	osm_mcast_req_type_t req_type = p_ctxt->req_type;
> >>>> -	ib_net64_t port_guid = p_ctxt->port_guid;
> >>>> -
> >>>> -	OSM_LOG_ENTER(p_mgr->p_log, osm_mcast_mgr_process_mgrp_cb);
> >>>> -
> >>>> -	/* nice copy no warning on size diff */
> >>>> -	memcpy(&mlid, &p_ctxt->mlid, sizeof(mlid));
> >>>> +	osm_signal_t ret, signal = OSM_SIGNAL_DONE;
> >>>> +	osm_mcast_mgr_ctxt_t *ctx;
> >>>> +	osm_mcast_req_type_t req_type;
> >>>> +	ib_net64_t port_guid;
> >>>>  -	/* we can destroy the context now */
> >>>> -	free(p_ctxt);
> >>>> +	OSM_LOG_ENTER(p_mgr->p_log, osm_mcast_mgr_process_mgroups);
> >>>>   	/* we need a lock to make sure the p_mgrp is not change other ways */
> >>>>  	CL_PLOCK_EXCL_ACQUIRE(p_mgr->p_lock);
> >>>> -	p_mgrp = __get_mgrp_by_mlid(p_mgr, mlid);
> >>>>  -	/* since we delayed the execution we prefer to pass the
> >>>> -	   mlid as the mgrp identifier and then find it or abort */
> >>>> +	if (cl_is_qlist_empty(p_list)) {
> >>>> +		CL_PLOCK_RELEASE(p_mgr->p_lock);
> >>>> +		return OSM_SIGNAL_NONE;
> >>>> +	}
> >>>> +
> >>>> +	while (!cl_is_qlist_empty(p_list)) {
> >>>> +		ctx = (osm_mcast_mgr_ctxt_t *) cl_qlist_remove_head(p_list);
> >>>> +		req_type = ctx->req_type;
> >>>> +		port_guid = ctx->port_guid;
> >>>> +
> >>>> +		/* nice copy no warning on size diff */
> >>>> +		memcpy(&mlid, &ctx->mlid, sizeof(mlid));
> >>>>  -	if (p_mgrp) {
> >>>> +		/* we can destroy the context now */
> >>>> +		free(ctx);
> >>>> +
> >>>> +		/* since we delayed the execution we prefer to pass the
> >>>> +		   mlid as the mgrp identifier and then find it or abort */
> >>>> +		p_mgrp = __get_mgrp_by_mlid(p_mgr, mlid);
> >>>> +		if (!p_mgrp)
> >>>> +			continue;
> >>>>  -		/* if there was no change from the last time we processed the group
> >>>> -		   we can skip doing anything
> >>>> +		/* if there was no change from the last time
> >>>> +		 * we processed the group we can skip doing anything
> >>>>  		 */
> >>>>  		if (p_mgrp->last_change_id == p_mgrp->last_tree_id) {
> >>>>  			osm_log(p_mgr->p_log, OSM_LOG_DEBUG,
> >>>> -				"osm_mcast_mgr_process_mgrp_cb: "
> >>>> +				"osm_mcast_mgr_process_mgroups: "
> >>>>  				"Skip processing mgrp with lid:0x%X change id:%u\n",
> >>>>  				cl_ntoh16(mlid), p_mgrp->last_change_id);
> >>>> -		} else {
> >>>> -			osm_log(p_mgr->p_log, OSM_LOG_DEBUG,
> >>>> -				"osm_mcast_mgr_process_mgrp_cb: "
> >>>> -				"Processing mgrp with lid:0x%X change id:%u\n",
> >>>> -				cl_ntoh16(mlid), p_mgrp->last_change_id);
> >>>> -
> >>>> -			signal =
> >>>> -			    osm_mcast_mgr_process_mgrp(p_mgr, p_mgrp, req_type,
> >>>> -						       port_guid);
> >>>> -			p_mgrp->last_tree_id = p_mgrp->last_change_id;
> >>>> +			continue;
> >>>>  		}
> >>>>  -		/* Remove MGRP only if osm_mcm_port_t count is 0 and
> >>>> -		 * Not a well known group
> >>>> -		 */
> >>>> -		if ((0x0 == cl_qmap_count(&p_mgrp->mcm_port_tbl)) &&
> >>>> -		    (p_mgrp->well_known == FALSE)) {
> >>>> -			osm_log(p_mgr->p_log, OSM_LOG_DEBUG,
> >>>> -				"osm_mcast_mgr_process_mgrp_cb: "
> >>>> -				"Destroying mgrp with lid:0x%X\n",
> >>>> -				cl_ntoh16(mlid));
> >>>> -
> >>>> -			/* Send a Report to any InformInfo registered for
> >>>> -			   Trap 67 : MCGroup delete */
> >>>> -			osm_mgrp_send_delete_notice(p_mgr->p_subn, p_mgr->p_log,
> >>>> -						    p_mgrp);
> >>>> -
> >>>> -			cl_qmap_remove_item(&p_mgr->p_subn->mgrp_mlid_tbl,
> >>>> -					    (cl_map_item_t *) p_mgrp);
> >>>> +		osm_log(p_mgr->p_log, OSM_LOG_DEBUG,
> >>>> +			"osm_mcast_mgr_process_mgroups: "
> >>>> +			"Processing mgrp with lid:0x%X change id:%u\n",
> >>>> +			cl_ntoh16(mlid), p_mgrp->last_change_id);
> >>>> +		mcast_mgr_process_mgrp(p_mgr, p_mgrp, req_type, port_guid);
> >>>> +	}
> >>>>  -			osm_mgrp_delete(p_mgrp);
> >>>> -		}
> >>>> +	/*
> >>>> +	   Walk the switches and download the tables for each.
> >>>> +	 */
> >>>> +	p_sw_tbl = &p_mgr->p_subn->sw_guid_tbl;
> >>>> +	p_sw = (osm_switch_t *) cl_qmap_head(p_sw_tbl);
> >>>> +	while (p_sw != (osm_switch_t *) cl_qmap_end(p_sw_tbl)) {
> >>>> +		ret = __osm_mcast_mgr_set_tbl(p_mgr, p_sw);
> >>>> +		if (ret == OSM_SIGNAL_DONE_PENDING)
> >>>> +			signal = ret;
> >>>> +		p_sw = (osm_switch_t *) cl_qmap_next(&p_sw->map_item);
> >>>>  	}
> >>>>  +	osm_dump_mcast_routes(p_mgr->p_subn->p_osm);
> >>>> +
> >>>>  	CL_PLOCK_RELEASE(p_mgr->p_lock);
> >>>>  	OSM_LOG_EXIT(p_mgr->p_log);
> >>>>  	return signal;
> >>>> diff --git a/opensm/opensm/osm_sm.c b/opensm/opensm/osm_sm.c
> >>>> index 88e6d4a..b295a77 100644
> >>>> --- a/opensm/opensm/osm_sm.c
> >>>> +++ b/opensm/opensm/osm_sm.c
> >>>> @@ -144,6 +144,7 @@ void osm_sm_construct(IN osm_sm_t * const p_sm)
> >>>>  	cl_event_construct(&p_sm->signal_event);
> >>>>  	cl_event_construct(&p_sm->subnet_up_event);
> >>>>  	cl_thread_construct(&p_sm->sweeper);
> >>>> +	cl_spinlock_construct(&p_sm->mgrp_lock);
> >>>>  	osm_req_construct(&p_sm->req);
> >>>>  	osm_resp_construct(&p_sm->resp);
> >>>>  	osm_ni_rcv_construct(&p_sm->ni_rcv);
> >>>> @@ -245,6 +246,7 @@ void osm_sm_destroy(IN osm_sm_t * const p_sm)
> >>>>  	cl_event_destroy(&p_sm->signal_event);
> >>>>  	cl_event_destroy(&p_sm->subnet_up_event);
> >>>>  	cl_spinlock_destroy(&p_sm->signal_lock);
> >>>> +	cl_spinlock_destroy(&p_sm->mgrp_lock);
> >>>>   	osm_log(p_sm->p_log, OSM_LOG_SYS, "Exiting SM\n");	/* Format Waived 
> >>>> */
> >>>>  	OSM_LOG_EXIT(p_sm->p_log);
> >>>> @@ -292,6 +294,12 @@ osm_sm_init(IN osm_sm_t * const p_sm,
> >>>>  	if (status != CL_SUCCESS)
> >>>>  		goto Exit;
> >>>>  +	cl_qlist_init(&p_sm->mgrp_list);
> >>>> +
> >>>> +	status = cl_spinlock_init(&p_sm->mgrp_lock);
> >>>> +	if (status != CL_SUCCESS)
> >>>> +		goto Exit;
> >>>> +
> >>>>  	status = osm_sm_mad_ctrl_init(&p_sm->mad_ctrl,
> >>>>  				      p_sm->p_subn,
> >>>>  				      p_sm->p_mad_pool,
> >>>> @@ -551,32 +559,43 @@ osm_sm_bind(IN osm_sm_t * const p_sm, IN const 
> >>>> ib_net64_t port_guid)
> >>>>  /**********************************************************************
> >>>>   
> >>>> **********************************************************************/
> >>>>  static ib_api_status_t
> >>>> -__osm_sm_mgrp_connect(IN osm_sm_t * const p_sm,
> >>>> +__osm_sm_mgrp_process(IN osm_sm_t * const p_sm,
> >>>>  		      IN osm_mgrp_t * const p_mgrp,
> >>>>  		      IN const ib_net64_t port_guid,
> >>>>  		      IN osm_mcast_req_type_t req_type)
> >>>>  {
> >>>> -	ib_api_status_t status;
> >>>>  	osm_mcast_mgr_ctxt_t *ctx2;
> >>>>  -	OSM_LOG_ENTER(p_sm->p_log, __osm_sm_mgrp_connect);
> >>>> -
> >>>>  	/*
> >>>>  	 * 'Schedule' all the QP0 traffic for when the state manager
> >>>>  	 * isn't busy trying to do something else.
> >>>>  	 */
> >>>>  	ctx2 = (osm_mcast_mgr_ctxt_t *) malloc(sizeof(osm_mcast_mgr_ctxt_t));
> >>>> +	if (!ctx2)
> >>>> +		return IB_ERROR;
> >>>> +	memset(ctx2, 0, sizeof(*ctx2));
> >>>>  	memcpy(&ctx2->mlid, &p_mgrp->mlid, sizeof(p_mgrp->mlid));
> >>>>  	ctx2->req_type = req_type;
> >>>>  	ctx2->port_guid = port_guid;
> >>>>  -	status = osm_state_mgr_process_idle(&p_sm->state_mgr,
> >>>> -					    osm_mcast_mgr_process_mgrp_cb,
> >>>> -					    NULL, &p_sm->mcast_mgr,
> >>>> -					    (void *)ctx2);
> >>>> +	cl_spinlock_acquire(&p_sm->mgrp_lock);
> >>>> +	cl_qlist_insert_tail(&p_sm->mgrp_list, &ctx2->list_item);
> >>>> +	cl_spinlock_release(&p_sm->mgrp_lock);
> >>>>  -	OSM_LOG_EXIT(p_sm->p_log);
> >>>> -	return (status);
> >>>> +	osm_sm_signal(p_sm, OSM_SIGNAL_IDLE_TIME_PROCESS_REQUEST);
> >>>> +
> >>>> +	return IB_SUCCESS;
> >>>> +}
> >>>> +
> >>>> +/**********************************************************************
> >>>> + 
> >>>> **********************************************************************/
> >>>> +static ib_api_status_t
> >>>> +__osm_sm_mgrp_connect(IN osm_sm_t * const p_sm,
> >>>> +		      IN osm_mgrp_t * const p_mgrp,
> >>>> +		      IN const ib_net64_t port_guid,
> >>>> +		      IN osm_mcast_req_type_t req_type)
> >>>> +{
> >>>> +	return __osm_sm_mgrp_process(p_sm, p_mgrp, port_guid, req_type);
> >>>>  }
> >>>>   
> >>>> /**********************************************************************
> >>>> @@ -586,31 +605,7 @@ __osm_sm_mgrp_disconnect(IN osm_sm_t * const p_sm,
> >>>>  			 IN osm_mgrp_t * const p_mgrp,
> >>>>  			 IN const ib_net64_t port_guid)
> >>>>  {
> >>>> -	ib_api_status_t status;
> >>>> -	osm_mcast_mgr_ctxt_t *ctx2;
> >>>> -
> >>>> -	OSM_LOG_ENTER(p_sm->p_log, __osm_sm_mgrp_disconnect);
> >>>> -
> >>>> -	/*
> >>>> -	 * 'Schedule' all the QP0 traffic for when the state manager
> >>>> -	 * isn't busy trying to do something else.
> >>>> -	 */
> >>>> -	ctx2 = (osm_mcast_mgr_ctxt_t *) malloc(sizeof(osm_mcast_mgr_ctxt_t));
> >>>> -	memcpy(&ctx2->mlid, &p_mgrp->mlid, sizeof(p_mgrp->mlid));
> >>>> -	ctx2->req_type = OSM_MCAST_REQ_TYPE_LEAVE;
> >>>> -	ctx2->port_guid = port_guid;
> >>>> -
> >>>> -	status = osm_state_mgr_process_idle(&p_sm->state_mgr,
> >>>> -					    osm_mcast_mgr_process_mgrp_cb,
> >>>> -					    NULL, &p_sm->mcast_mgr, ctx2);
> >>>> -	if (status != IB_SUCCESS) {
> >>>> -		osm_log(p_sm->p_log, OSM_LOG_ERROR,
> >>>> -			"__osm_sm_mgrp_disconnect: ERR 2E11: "
> >>>> -			"Failure processing multicast group (%s)\n",
> >>>> -			ib_get_err_str(status));
> >>>> -	}
> >>>> -
> >>>> -	OSM_LOG_EXIT(p_sm->p_log);
> >>>> +	__osm_sm_mgrp_process(p_sm, p_mgrp, port_guid, 
> >>>> OSM_MCAST_REQ_TYPE_LEAVE);
> >>>>  }
> >>>>   
> >>>> /**********************************************************************
> >>>> @@ -719,8 +714,8 @@ osm_sm_mcgrp_join(IN osm_sm_t * const p_sm,
> >>>>  		goto Exit;
> >>>>  	}
> >>>>  -	CL_PLOCK_RELEASE(p_sm->p_lock);
> >>>>  	status = __osm_sm_mgrp_connect(p_sm, p_mgrp, port_guid, req_type);
> >>>> +	CL_PLOCK_RELEASE(p_sm->p_lock);
> >>>>         Exit:
> >>>>  	OSM_LOG_EXIT(p_sm->p_log);
> >>>> @@ -782,9 +777,8 @@ osm_sm_mcgrp_leave(IN osm_sm_t * const p_sm,
> >>>>   	osm_port_remove_mgrp(p_port, mlid);
> >>>>  -	CL_PLOCK_RELEASE(p_sm->p_lock);
> >>>> -
> >>>>  	__osm_sm_mgrp_disconnect(p_sm, p_mgrp, port_guid);
> >>>> +	CL_PLOCK_RELEASE(p_sm->p_lock);
> >>>>         Exit:
> >>>>  	OSM_LOG_EXIT(p_sm->p_log);
> >>>> diff --git a/opensm/opensm/osm_state_mgr.c 
> >>>> b/opensm/opensm/osm_state_mgr.c
> >>>> index 5c39f11..d4dd782 100644
> >>>> --- a/opensm/opensm/osm_state_mgr.c
> >>>> +++ b/opensm/opensm/osm_state_mgr.c
> >>>> @@ -76,7 +76,6 @@ osm_signal_t osm_qos_setup(IN osm_opensm_t * p_osm);
> >>>>  void osm_state_mgr_construct(IN osm_state_mgr_t * const p_mgr)
> >>>>  {
> >>>>  	memset(p_mgr, 0, sizeof(*p_mgr));
> >>>> -	cl_spinlock_construct(&p_mgr->idle_lock);
> >>>>  	p_mgr->state = OSM_SM_STATE_INIT;
> >>>>  }
> >>>>  @@ -88,9 +87,6 @@ void osm_state_mgr_destroy(IN osm_state_mgr_t * const 
> >>>> p_mgr)
> >>>>   	OSM_LOG_ENTER(p_mgr->p_log, osm_state_mgr_destroy);
> >>>>  -	/* destroy the locks */
> >>>> -	cl_spinlock_destroy(&p_mgr->idle_lock);
> >>>> -
> >>>>  	OSM_LOG_EXIT(p_mgr->p_log);
> >>>>  }
> >>>>  @@ -112,8 +108,6 @@ osm_state_mgr_init(IN osm_state_mgr_t * const 
> >>>> p_mgr,
> >>>>  		   IN cl_event_t * const p_subnet_up_event,
> >>>>  		   IN osm_log_t * const p_log)
> >>>>  {
> >>>> -	cl_status_t status;
> >>>> -
> >>>>  	OSM_LOG_ENTER(p_log, osm_state_mgr_init);
> >>>>   	CL_ASSERT(p_subn);
> >>>> @@ -145,17 +139,8 @@ osm_state_mgr_init(IN osm_state_mgr_t * const 
> >>>> p_mgr,
> >>>>  	p_mgr->p_lock = p_lock;
> >>>>  	p_mgr->p_subnet_up_event = p_subnet_up_event;
> >>>>  -	cl_qlist_init(&p_mgr->idle_time_list);
> >>>> -
> >>>> -	status = cl_spinlock_init(&p_mgr->idle_lock);
> >>>> -	if (status != CL_SUCCESS) {
> >>>> -		osm_log(p_mgr->p_log, OSM_LOG_ERROR,
> >>>> -			"osm_state_mgr_init: ERR 3302: "
> >>>> -			"Spinlock init failed (%s)\n", CL_STATUS_MSG(status));
> >>>> -	}
> >>>> -
> >>>>  	OSM_LOG_EXIT(p_mgr->p_log);
> >>>> -	return (status);
> >>>> +	return IB_SUCCESS;
> >>>>  }
> >>>>   
> >>>> /**********************************************************************
> >>>> @@ -989,79 +974,6 @@ static ib_api_status_t 
> >>>> __osm_state_mgr_light_sweep_start(IN osm_state_mgr_t *
> >>>>  }
> >>>>   
> >>>> /**********************************************************************
> >>>> - 
> >>>> **********************************************************************/
> >>>> -static void __process_idle_time_queue_done(IN osm_state_mgr_t * const 
> >>>> p_mgr)
> >>>> -{
> >>>> -	cl_qlist_t *p_list = &p_mgr->idle_time_list;
> >>>> -	cl_list_item_t *p_list_item;
> >>>> -	osm_idle_item_t *p_process_item;
> >>>> -
> >>>> -	OSM_LOG_ENTER(p_mgr->p_log, __process_idle_time_queue_done);
> >>>> -
> >>>> -	cl_spinlock_acquire(&p_mgr->idle_lock);
> >>>> -	p_list_item = cl_qlist_remove_head(p_list);
> >>>> -
> >>>> -	if (p_list_item == cl_qlist_end(p_list)) {
> >>>> -		cl_spinlock_release(&p_mgr->idle_lock);
> >>>> -		osm_log(p_mgr->p_log, OSM_LOG_ERROR,
> >>>> -			"__process_idle_time_queue_done: ERR 3314: "
> >>>> -			"Idle time queue is empty\n");
> >>>> -		return;
> >>>> -	}
> >>>> -	cl_spinlock_release(&p_mgr->idle_lock);
> >>>> -
> >>>> -	p_process_item = (osm_idle_item_t *) p_list_item;
> >>>> -
> >>>> -	if (p_process_item->pfn_done) {
> >>>> -
> >>>> -		p_process_item->pfn_done(p_process_item->context1,
> >>>> -					 p_process_item->context2);
> >>>> -	}
> >>>> -
> >>>> -	free(p_process_item);
> >>>> -
> >>>> -	OSM_LOG_EXIT(p_mgr->p_log);
> >>>> -	return;
> >>>> -}
> >>>> -
> >>>> -/**********************************************************************
> >>>> - 
> >>>> **********************************************************************/
> >>>> -static osm_signal_t __process_idle_time_queue_start(IN osm_state_mgr_t 
> >>>> *
> >>>> -						    const p_mgr)
> >>>> -{
> >>>> -	cl_qlist_t *p_list = &p_mgr->idle_time_list;
> >>>> -	cl_list_item_t *p_list_item;
> >>>> -	osm_idle_item_t *p_process_item;
> >>>> -	osm_signal_t signal;
> >>>> -
> >>>> -	OSM_LOG_ENTER(p_mgr->p_log, __process_idle_time_queue_start);
> >>>> -
> >>>> -	cl_spinlock_acquire(&p_mgr->idle_lock);
> >>>> -
> >>>> -	p_list_item = cl_qlist_head(p_list);
> >>>> -	if (p_list_item == cl_qlist_end(p_list)) {
> >>>> -		cl_spinlock_release(&p_mgr->idle_lock);
> >>>> -		OSM_LOG_EXIT(p_mgr->p_log);
> >>>> -		return OSM_SIGNAL_NONE;
> >>>> -	}
> >>>> -
> >>>> -	cl_spinlock_release(&p_mgr->idle_lock);
> >>>> -
> >>>> -	p_process_item = (osm_idle_item_t *) p_list_item;
> >>>> -
> >>>> -	CL_ASSERT(p_process_item->pfn_start);
> >>>> -
> >>>> -	signal =
> >>>> -	    p_process_item->pfn_start(p_process_item->context1,
> >>>> -				      p_process_item->context2);
> >>>> -
> >>>> -	CL_ASSERT(signal != OSM_SIGNAL_NONE);
> >>>> -
> >>>> -	OSM_LOG_EXIT(p_mgr->p_log);
> >>>> -	return signal;
> >>>> -}
> >>>> -
> >>>> -/**********************************************************************
> >>>>   * Go over all the remote SMs (as updated in the sm_guid_tbl).
> >>>>   * Find if there is a remote sm that is a master SM.
> >>>>   * If there is a remote master SM - return a pointer to it,
> >>>> @@ -1558,7 +1470,7 @@ void osm_state_mgr_process(IN osm_state_mgr_t * 
> >>>> const p_mgr,
> >>>>  		case OSM_SM_STATE_PROCESS_REQUEST:
> >>>>  			switch (signal) {
> >>>>  			case OSM_SIGNAL_IDLE_TIME_PROCESS:
> >>>> -				signal = __process_idle_time_queue_start(p_mgr);
> >>>> +				signal = osm_mcast_mgr_process_mgroups(p_mgr->p_mcast_mgr);
> >>>>  				switch (signal) {
> >>>>  				case OSM_SIGNAL_NONE:
> >>>>  					p_mgr->state = OSM_SM_STATE_IDLE;
> >>>> @@ -1604,14 +1516,6 @@ void osm_state_mgr_process(IN osm_state_mgr_t * 
> >>>> const p_mgr,
> >>>>  			switch (signal) {
> >>>>  			case OSM_SIGNAL_NO_PENDING_TRANSACTIONS:
> >>>>  			case OSM_SIGNAL_DONE:
> >>>> -				/* CALL the done function */
> >>>> -				__process_idle_time_queue_done(p_mgr);
> >>>> -
> >>>> -				/*
> >>>> -				 * Set the signal to OSM_SIGNAL_IDLE_TIME_PROCESS
> >>>> -				 * so that the next element in the queue gets processed
> >>>> -				 */
> >>>> -
> >>>>  				signal = OSM_SIGNAL_IDLE_TIME_PROCESS;
> >>>>  				p_mgr->state = OSM_SM_STATE_PROCESS_REQUEST;
> >>>>  				break;
> >>>> @@ -2424,41 +2328,3 @@ void osm_state_mgr_process(IN osm_state_mgr_t * 
> >>>> const p_mgr,
> >>>>   	OSM_LOG_EXIT(p_mgr->p_log);
> >>>>  }
> >>>> -
> >>>> -/**********************************************************************
> >>>> - 
> >>>> **********************************************************************/
> >>>> -ib_api_status_t
> >>>> -osm_state_mgr_process_idle(IN osm_state_mgr_t * const p_mgr,
> >>>> -			   IN osm_pfn_start_t pfn_start,
> >>>> -			   IN osm_pfn_done_t pfn_done, void *context1,
> >>>> -			   void *context2)
> >>>> -{
> >>>> -	osm_idle_item_t *p_idle_item;
> >>>> -
> >>>> -	OSM_LOG_ENTER(p_mgr->p_log, osm_state_mgr_process_idle);
> >>>> -
> >>>> -	p_idle_item = malloc(sizeof(osm_idle_item_t));
> >>>> -	if (p_idle_item == NULL) {
> >>>> -		osm_log(p_mgr->p_log, OSM_LOG_ERROR,
> >>>> -			"osm_state_mgr_process_idle: ERR 3321: "
> >>>> -			"insufficient memory\n");
> >>>> -		return IB_ERROR;
> >>>> -	}
> >>>> -
> >>>> -	memset(p_idle_item, 0, sizeof(osm_idle_item_t));
> >>>> -	p_idle_item->pfn_start = pfn_start;
> >>>> -	p_idle_item->pfn_done = pfn_done;
> >>>> -	p_idle_item->context1 = context1;
> >>>> -	p_idle_item->context2 = context2;
> >>>> -
> >>>> -	cl_spinlock_acquire(&p_mgr->idle_lock);
> >>>> -	cl_qlist_insert_tail(&p_mgr->idle_time_list, &p_idle_item->list_item);
> >>>> -	cl_spinlock_release(&p_mgr->idle_lock);
> >>>> -
> >>>> -	osm_sm_signal(&p_mgr->p_subn->p_osm->sm,
> >>>> -		      OSM_SIGNAL_IDLE_TIME_PROCESS_REQUEST);
> >>>> -
> >>>> -	OSM_LOG_EXIT(p_mgr->p_log);
> >>>> -
> >>>> -	return IB_SUCCESS;
> >>>> -}
> >> _______________________________________________
> >> general mailing list
> >> general at lists.openfabrics.org
> >> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> >>
> >> To unsubscribe, please visit 
> >> http://openib.org/mailman/listinfo/openib-general
> > _______________________________________________
> > general mailing list
> > general at lists.openfabrics.org
> > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> > To unsubscribe, please visit 
> > http://openib.org/mailman/listinfo/openib-general
> 



More information about the general mailing list