[openib-general] [PATCH] CM: check cm_id state before handling a REP

Sean Hefty sean.hefty at intel.com
Thu Mar 2 11:48:56 PST 2006


Move checking the state of a cm_id before modifying it when handling a REP.

Signed-off-by: Sean Hefty <sean.hefty at intel.com>

---

This fixes a bug accessing a NULL timewait_info pointer if a request times
out before a REP is received that was seen under MPI scale-up testing.

 
Index: cm.c
===================================================================
--- cm.c	(revision 5552)
+++ cm.c	(working copy)
@@ -121,7 +121,7 @@ struct cm_id_private {
 
 	struct rb_node service_node;
 	struct rb_node sidr_id_node;
-	spinlock_t lock;
+	spinlock_t lock;	/* Do not acquire inside cm.lock */
 	wait_queue_head_t wait;
 	atomic_t refcount;
 
@@ -1613,6 +1613,19 @@ static int cm_rep_handler(struct cm_work
 		return -EINVAL;
 	}
 
+	cm_format_rep_event(work);
+
+	spin_lock_irqsave(&cm_id_priv->lock, flags);
+	switch (cm_id_priv->id.state) {
+	case IB_CM_REQ_SENT:
+	case IB_CM_MRA_REQ_RCVD:
+		break;
+	default:
+		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
+		ret = -EINVAL;
+		goto error;
+	}
+
 	cm_id_priv->timewait_info->work.remote_id = rep_msg->local_comm_id;
 	cm_id_priv->timewait_info->remote_ca_guid = rep_msg->local_ca_guid;
 	cm_id_priv->timewait_info->remote_qpn = cm_rep_get_local_qpn(rep_msg);
@@ -1621,12 +1634,17 @@ static int cm_rep_handler(struct cm_work
 	/* Check for duplicate REP. */
 	if (cm_insert_remote_id(cm_id_priv->timewait_info)) {
 		spin_unlock_irqrestore(&cm.lock, flags);
+		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
 		ret = -EINVAL;
 		goto error;
 	}
 	/* Check for a stale connection. */
 	if (cm_insert_remote_qpn(cm_id_priv->timewait_info)) {
+		rb_erase(&cm_id_priv->timewait_info->remote_id_node,
+			 &cm.remote_id_table);
+		cm_id_priv->timewait_info->inserted_remote_id = 0;
 		spin_unlock_irqrestore(&cm.lock, flags);
+		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
 		cm_issue_rej(work->port, work->mad_recv_wc,
 			     IB_CM_REJ_STALE_CONN, CM_MSG_RESPONSE_REP,
 			     NULL, 0);
@@ -1635,18 +1653,6 @@ static int cm_rep_handler(struct cm_work
 	}
 	spin_unlock_irqrestore(&cm.lock, flags);
 
-	cm_format_rep_event(work);
-
-	spin_lock_irqsave(&cm_id_priv->lock, flags);
-	switch (cm_id_priv->id.state) {
-	case IB_CM_REQ_SENT:
-	case IB_CM_MRA_REQ_RCVD:
-		break;
-	default:
-		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
-		ret = -EINVAL;
-		goto error;
-	}
 	cm_id_priv->id.state = IB_CM_REP_RCVD;
 	cm_id_priv->id.remote_id = rep_msg->local_comm_id;
 	cm_id_priv->remote_qpn = cm_rep_get_local_qpn(rep_msg);
@@ -1669,7 +1675,7 @@ static int cm_rep_handler(struct cm_work
 		cm_deref_id(cm_id_priv);
 	return 0;
 
-error:	cm_cleanup_timewait(cm_id_priv->timewait_info);
+error:
 	cm_deref_id(cm_id_priv);
 	return ret;
 }






More information about the general mailing list