Skip to contentSkip to conversation

Add separate dispatcher for SA set and delete requests

PATCH New Hal Rosenstock 1 participant 1 message Jan 5, 2015 InfiniBand

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add separate dispatcher for SA set and delete requests

From: Daniel Klein <danielk-VPRAkNaXOzVWk0Htik3J/[email protected]>

Add a separate dispatcher for SA set/delete requests in order to avoid dependent request race conditions. This is caused by SA client misbehavior (in not following IBA spec) in terms of dependent management requests.

Without this change, when an agent sends a set of requests, OpenSM might handle them in different order from that they were received, and if the requests are dependent on each other, the outcome may vary. With this change, requests will be handled in the order they were received.

Stress test result:
Test is to send 200K set/delete guid info requests in parallel (20 requests on the wire)
Without the patches: ~5.37 seconds
With the patches: 5.1 seconds

Background:
In terms of the general requirement/problem, the confusion stems from the IBA management "non standard" meaning of a transaction. A transaction ID is required to be unique if new operation (not rerequest of existing operation) where transaction is determined from the combination of TID, SGID (or SLID), and MgmtClass. There is no requirement about handling the ordering of transactions. This was explicitly omitted by design.
This lack of requirement actually goes further than just sets and could serialize all incoming SA transactions if done across clients.

This is a bug but it is not OpenSM misbehavior. The requirement on any ordering is on the client and not on the manager. The problem is to "deal with non compliant SA clients" without negatively impacting SA performance.

The specific issue is the ordering of set processing from a specific application in a node which is a subset of the above. If a client is to pipeline SA sets, it needs to know what it's doing and serialize subsequent sets which depend on previous ones. If that is not done (which is the case), per IBA standard, the bug is in the SA client.

---
 include/opensm/osm_opensm.h      |    8 +++++
 include/opensm/osm_sa.h          |   12 +++++++
 include/opensm/osm_sa_mad_ctrl.h |   14 ++++++++-
 opensm/osm_opensm.c              |   17 ++++++++++-
 opensm/osm_sa.c                  |   45 ++++++++++++++++++++++++++-
 opensm/osm_sa_mad_ctrl.c         |   61 +++++++++++++++++++++++++++++++------
 6 files changed, 143 insertions(+), 14 deletions(-)
include/opensm/osm_opensm.h +8 -0
index 4750505..7768881 100644
@@ -226,6 +226,8 @@ typedef struct osm_opensm {
 	osm_vl15_t vl15;
 	osm_log_t log;
 	cl_dispatcher_t disp;
+	cl_dispatcher_t sa_set_disp;
+	boolean_t sa_set_disp_initialized;
 	cl_plock_t lock;
 	struct osm_routing_engine *routing_engine_list;
 	struct osm_routing_engine *routing_engine_used;
@@ -269,6 +271,12 @@ typedef struct osm_opensm {
 *	disp
 *		Central dispatcher containing the OpenSM worker threads.
 *
+*	sa_set_disp
+*		Dispatcher for SA Set and Delete requests.
+*
+*	sa_set_disp_initialized.
+*		Indicator that sa_set_disp dispatcher was initialized.
+*
 *	lock
 *		Shared lock guarding most OpenSM structures.
 *
include/opensm/osm_sa.h +12 -0
index f9f334e..aeeaa02 100644
@@ -186,6 +186,7 @@ typedef struct osm_sa {
 	osm_log_t *p_log;
 	osm_mad_pool_t *p_mad_pool;
 	cl_dispatcher_t *p_disp;
+	cl_dispatcher_t *p_set_disp;
 	cl_plock_t *p_lock;
 	atomic32_t sa_trans_id;
 	osm_sa_mad_ctrl_t mad_ctrl;
@@ -211,6 +212,10 @@ typedef struct osm_sa {
 	cl_disp_reg_handle_t lft_disp_h;
 	cl_disp_reg_handle_t sir_disp_h;
 	cl_disp_reg_handle_t mft_disp_h;
+	cl_disp_reg_handle_t infr_set_disp_h;
+	cl_disp_reg_handle_t gir_set_disp_h;
+	cl_disp_reg_handle_t mcmr_set_disp_h;
+	cl_disp_reg_handle_t sr_set_disp_h;
 } osm_sa_t;
 /*
 * FIELDS
@@ -235,6 +240,9 @@ typedef struct osm_sa {
 *	p_disp
 *		Pointer to dispatcher
 *
+*	p_set_disp
+*		Pointer to dispatcher for Set requests.
+*
 *	p_lock
 *		Pointer to Lock for serialization
 *
@@ -346,6 +354,7 @@ ib_api_status_t osm_sa_init(IN osm_sm_t * p_sm, IN osm_sa_t * p_sa,
 			    IN osm_mad_pool_t * p_mad_pool,
 			    IN osm_log_t * p_log, IN osm_stats_t * p_stats,
 			    IN cl_dispatcher_t * p_disp,
+			    IN cl_dispatcher_t * p_set_disp,
 			    IN cl_plock_t * p_lock);
 /*
 * PARAMETERS
@@ -370,6 +379,9 @@ ib_api_status_t osm_sa_init(IN osm_sm_t * p_sm, IN osm_sa_t * p_sa,
 *	p_disp
 *		[in] Pointer to the OpenSM central Dispatcher.
 *
+*	p_set_disp
+*		[in] Pointer to the OpenSM Dispatcher for Set requests.
+*
 *	p_lock
 *		[in] Pointer to the OpenSM serializing lock.
 *
include/opensm/osm_sa_mad_ctrl.h +13 -1
index 00ccbf6..2c2513b 100644
@@ -98,7 +98,9 @@ typedef struct osm_sa_mad_ctrl {
 	osm_vendor_t *p_vendor;
 	osm_bind_handle_t h_bind;
 	cl_dispatcher_t *p_disp;
+	cl_dispatcher_t *p_set_disp;
 	cl_disp_reg_handle_t h_disp;
+	cl_disp_reg_handle_t h_set_disp;
 	osm_stats_t *p_stats;
 	osm_subn_t *p_subn;
 } osm_sa_mad_ctrl_t;
@@ -122,9 +124,15 @@ typedef struct osm_sa_mad_ctrl {
 *	p_disp
 *		Pointer to the Dispatcher.
 *
+*	p_set_disp
+*		Pointer to the Dispatcher for Set requests.
+*
 *	h_disp
 *		Handle returned from dispatcher registration.
 *
+*	h_set_disp
+*		Handle returned from Set requests dispatcher registration.
+*
 *	p_stats
 *		Pointer to the OpenSM statistics block.
 *
@@ -211,7 +219,8 @@ ib_api_status_t osm_sa_mad_ctrl_init(IN osm_sa_mad_ctrl_t * p_ctrl,
 				     IN osm_subn_t * p_subn,
 				     IN osm_log_t * p_log,
 				     IN osm_stats_t * p_stats,
-				     IN cl_dispatcher_t * p_disp);
+				     IN cl_dispatcher_t * p_disp,
+				     IN cl_dispatcher_t * p_set_disp);
 /*
 * PARAMETERS
 *	p_ctrl
@@ -235,6 +244,9 @@ ib_api_status_t osm_sa_mad_ctrl_init(IN osm_sa_mad_ctrl_t * p_ctrl,
 *	p_disp
 *		[in] Pointer to the OpenSM central Dispatcher.
 *
+*	p_set_disp
+*		[in] Pointer to the OpenSM Dispatcher for Set requests.
+*
 * RETURN VALUES
 *	IB_SUCCESS if the SA MAD Controller object was initialized
 *	successfully.
opensm/osm_opensm.c +16 -1
index 69d2ba6..afa2544 100644
@@ -345,6 +345,8 @@ void osm_opensm_destroy_finish(IN osm_opensm_t * p_osm)
 	osm_vendor_delete(&p_osm->p_vendor);
 	osm_subn_destroy(&p_osm->subn);
 	cl_disp_destroy(&p_osm->disp);
+	if (p_osm->sa_set_disp_initialized)
+		cl_disp_destroy(&p_osm->sa_set_disp);
 #ifdef HAVE_LIBPTHREAD
 	pthread_cond_destroy(&p_osm->stats.cond);
 	pthread_mutex_destroy(&p_osm->stats.mutex);
@@ -432,6 +434,17 @@ ib_api_status_t osm_opensm_init(IN osm_opensm_t * p_osm,
 	if (status != IB_SUCCESS)
 		goto Exit;
 
+	/* Unless OpenSM runs in single threaded mode, we create new single
+	 * threaded dispatcher for SA Set and Delete requets.
+	 */
+	p_osm->sa_set_disp_initialized = FALSE;
+	if (!p_opt->single_thread) {
+		status = cl_disp_init(&p_osm->sa_set_disp, 1, "subnadmin_set");
+		if (status != IB_SUCCESS)
+			goto Exit;
+		p_osm->sa_set_disp_initialized = TRUE;
+	}
+
 	/* the DB is in use by subn so init before */
 	status = osm_db_init(&p_osm->db, &p_osm->log);
 	if (status != IB_SUCCESS)
@@ -480,7 +493,9 @@ ib_api_status_t osm_opensm_init_finish(IN osm_opensm_t * p_osm,
 
 	status = osm_sa_init(&p_osm->sm, &p_osm->sa, &p_osm->subn,
 			     p_osm->p_vendor, &p_osm->mad_pool, &p_osm->log,
-			     &p_osm->stats, &p_osm->disp, &p_osm->lock);
+			     &p_osm->stats, &p_osm->disp,
+			     p_opt->single_thread ? NULL : &p_osm->sa_set_disp,
+			     &p_osm->lock);
 	if (status != IB_SUCCESS)
 		goto Exit;
 
opensm/osm_sa.c +43 -2

75 lines not on this page · raw message

opensm/osm_sa_mad_ctrl.c +51 -10

148 lines not on this page · raw message

Signature · 7 lines
1.7.1


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html