[ofa-general] [PATCH] opensm: fix race in main OpenSM flow.

[Sasha Khapyorsky]

wait_for_pending_transactions() is heavily used during OpenSM heavy and
light sweep, it checks opensm.stats.qp0_mads_outstanding value and blocks
(with pthread_cond_wait()) until it will reach zero, this event is
signaled by pthread_cond_signal() and should wake waiting thread up.
However this code (qp0_mads_outstanding decrease and signaling) is not
protected by the same mutex as check code in
wait_for_pending_transactions() is. As result there is an easily
reproducible race condition (when qp0_mads_outstanding is decreased and
signaled after the check and before pthread_cond_wait() call in
wait_for_pending_transactions()), which causes OpenSM to dead lock.

This patch addresses this issue - it will use the same mutex which is
used in wait_for_pending_transactions() for all qp0_mads_outstanding
change operations.

Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
---
 opensm/include/opensm/osm_stats.h |   34 ++++++++++++++++++++++++++++++++++
 opensm/opensm/osm_sm_mad_ctrl.c   |   21 +++++----------------
 opensm/opensm/osm_vl15intf.c      |    4 ++--
 3 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/opensm/include/opensm/osm_stats.h b/opensm/include/opensm/osm_stats.h
index 2b06e3f..4331cfa 100644
--- a/opensm/include/opensm/osm_stats.h
+++ b/opensm/include/opensm/osm_stats.h
@@ -146,5 +146,39 @@ typedef struct osm_stats {
 * SEE ALSO
 ***************/
 
+static inline uint32_t osm_stats_inc_qp0_outstanding(osm_stats_t *stats)
+{
+	uint32_t outstanding;
+
+#ifdef HAVE_LIBPTHREAD
+	pthread_mutex_lock(&stats->mutex);
+	outstanding = ++stats->qp0_mads_outstanding;
+	pthread_mutex_unlock(&stats->mutex);
+#else
+	outstanding = cl_atomic_inc(&stats->qp0_mads_outstanding);
+#endif
+
+	return outstanding;
+}
+
+static inline uint32_t osm_stats_dec_qp0_outstanding(osm_stats_t *stats)
+{
+	uint32_t outstanding;
+
+#ifdef HAVE_LIBPTHREAD
+	pthread_mutex_lock(&stats->mutex);
+	outstanding = --stats->qp0_mads_outstanding;
+	if (!outstanding)
+		pthread_cond_signal(&stats->cond);
+	pthread_mutex_unlock(&stats->mutex);
+#else
+	outstanding = cl_atomic_dec(&stats->qp0_mads_outstanding);
+	if (!outstanding)
+		cl_event_signal(&stats->event);
+#endif
+
+	return outstanding;
+}
+
 END_C_DECLS
 #endif				/* _OSM_STATS_H_ */
diff --git a/opensm/opensm/osm_sm_mad_ctrl.c b/opensm/opensm/osm_sm_mad_ctrl.c
index fa588cf..267ec85 100644
--- a/opensm/opensm/osm_sm_mad_ctrl.c
+++ b/opensm/opensm/osm_sm_mad_ctrl.c
@@ -64,6 +64,7 @@
  *
  * SYNOPSIS
  */
+
 static void
 __osm_sm_mad_ctrl_retire_trans_mad(IN osm_sm_mad_ctrl_t * const p_ctrl,
 				   IN osm_madw_t * const p_madw)
@@ -82,23 +83,11 @@ __osm_sm_mad_ctrl_retire_trans_mad(IN osm_sm_mad_ctrl_t * const p_ctrl,
 
 	osm_mad_pool_put(p_ctrl->p_mad_pool, p_madw);
 
-	outstanding = cl_atomic_dec(&p_ctrl->p_stats->qp0_mads_outstanding);
-
-	OSM_LOG(p_ctrl->p_log, OSM_LOG_DEBUG, "%u QP0 MADs outstanding\n",
-		p_ctrl->p_stats->qp0_mads_outstanding);
+	outstanding = osm_stats_dec_qp0_outstanding(p_ctrl->p_stats);
 
-	if (outstanding == 0) {
-		/*
-		   The wire is clean.
-		   Signal the subnet manager.
-		 */
-		OSM_LOG(p_ctrl->p_log, OSM_LOG_DEBUG, "wire is clean.\n");
-#ifdef HAVE_LIBPTHREAD
-		pthread_cond_signal(&p_ctrl->p_stats->cond);
-#else
-		cl_event_signal(&p_ctrl->p_stats->event);
-#endif
-	}
+	OSM_LOG(p_ctrl->p_log, OSM_LOG_DEBUG, "%u QP0 MADs outstanding%s\n",
+		p_ctrl->p_stats->qp0_mads_outstanding,
+		outstanding ? "" : ": wire is clean.");
 
 	OSM_LOG_EXIT(p_ctrl->p_log);
 }
diff --git a/opensm/opensm/osm_vl15intf.c b/opensm/opensm/osm_vl15intf.c
index 0cd88ec..0703a4f 100644
--- a/opensm/opensm/osm_vl15intf.c
+++ b/opensm/opensm/osm_vl15intf.c
@@ -325,7 +325,7 @@ void osm_vl15_post(IN osm_vl15_t * const p_vl, IN osm_madw_t * const p_madw)
 	cl_spinlock_acquire(&p_vl->lock);
 	if (p_madw->resp_expected == TRUE) {
 		cl_qlist_insert_tail(&p_vl->rfifo, &p_madw->list_item);
-		cl_atomic_inc(&p_vl->p_stats->qp0_mads_outstanding);
+		osm_stats_inc_qp0_outstanding(p_vl->p_stats);
 	} else
 		cl_qlist_insert_tail(&p_vl->ufifo, &p_madw->list_item);
 	cl_spinlock_release(&p_vl->lock);
@@ -374,7 +374,7 @@ osm_vl15_shutdown(IN osm_vl15_t * const p_vl,
 			"Releasing Request p_madw = %p\n", p_madw);
 
 		osm_mad_pool_put(p_mad_pool, p_madw);
-		cl_atomic_dec(&p_vl->p_stats->qp0_mads_outstanding);
+		osm_stats_dec_qp0_outstanding(p_vl->p_stats);
 
 		p_madw = (osm_madw_t *) cl_qlist_remove_head(&p_vl->rfifo);
 	}
-- 
1.6.1.rc1.45.g123ed