[openib-general] [PATCH 1/2] sdp_conn_put/sdp_conn_hold race condition

[Michael S. Tsirkin]

Protect sdp_conn_put by dev_root_s.sock_lock.
Add sdp_conn_put_light for when we know its not the last reference.

Signed-off-by: Michael S. Tsirkin <mst at mellanox.co.il>

Index: ulp/sdp/sdp_conn.c
===================================================================
--- ulp/sdp/sdp_conn.c	(revision 2784)
+++ ulp/sdp/sdp_conn.c	(working copy)
@@ -595,12 +595,6 @@ static int sdp_conn_table_insert(struct 
 int sdp_conn_table_remove(struct sdp_sock *conn)
 {
 	int result = 0;
-	unsigned long flags;
-
-	/*
-	 * lock table
-	 */
-	spin_lock_irqsave(&dev_root_s.sock_lock, flags);
 	/*
 	 * validate entry
 	 */
@@ -619,10 +613,6 @@ int sdp_conn_table_remove(struct sdp_soc
 	conn->hashent = SDP_DEV_SK_INVALID;
 
 done:
-	/*
-	 * unlock table
-	 */
-	spin_unlock_irqrestore(&dev_root_s.sock_lock, flags);
 	return result;
 }
 
@@ -706,13 +696,30 @@ void sdp_iocb_q_cancel_all(struct sdp_so
  */
 
 /*
- * sdp_conn_destruct - final destructor for connection.
+ * sdp_conn_put - reference counting and final destructor for connection.
  */
-void sdp_conn_destruct(struct sdp_sock *conn)
+void sdp_conn_put(struct sdp_sock *conn)
 {
+	unsigned long flags;
 	int dump = 0;
 	int result;
 
+
+	spin_lock_irqsave(&dev_root_s.sock_lock, flags);
+
+	if (!atomic_dec_and_test(&conn->refcnt)) {
+		spin_unlock_irqrestore(&dev_root_s.sock_lock, flags);
+		return;
+	}
+
+	result = sdp_conn_table_remove(conn);
+	if (result < 0)
+		sdp_dbg_warn(conn, "Error <%d> removing connection <%u:%u>",
+			     result, dev_root_s.sk_entry,
+			     dev_root_s.sk_size);
+
+	spin_unlock_irqrestore(&dev_root_s.sock_lock, flags);
+
 	sdp_dbg_ctrl(conn, "DESTRUCT. <%08x:%04x> <%08x:%04x> <%u:%u>",
 		     conn->src_addr, conn->src_port,
 		     conn->dst_addr, conn->dst_port,
@@ -721,14 +728,6 @@ void sdp_conn_destruct(struct sdp_sock *
 	 * If the socket is bound, return the port
 	 */
 	(void)sdp_inet_port_put(conn);
-	/*
-	 * remove connection from table
-	 */
-	result = sdp_conn_table_remove(conn);
-	if (result < 0)
-		sdp_dbg_warn(conn, "Error <%d> removing connection <%u:%u>",
-			     result, dev_root_s.sk_entry,
-			     dev_root_s.sk_size);
 
 	sdp_conn_stat_dump(conn);
 	/*
Index: ulp/sdp/sdp_conn.h
===================================================================
--- ulp/sdp/sdp_conn.h	(revision 2784)
+++ ulp/sdp/sdp_conn.h	(working copy)
@@ -431,7 +431,6 @@ struct sdp_sock {
 extern void sdp_conn_internal_lock(struct sdp_sock *conn, unsigned long *flags);
 extern void sdp_conn_internal_unlock(struct sdp_sock *conn);
 extern void sdp_conn_relock(struct sdp_sock *conn);
-extern void sdp_conn_destruct(struct sdp_sock *conn);
 extern int sdp_conn_cq_drain(struct ib_cq *cq, struct sdp_sock *conn);
 
 #define SDP_CONN_LOCK_IRQ(conn, flags) \
@@ -479,12 +478,17 @@ static inline void sdp_conn_hold(struct 
 	atomic_inc(&conn->refcnt);
 }
 
-static inline void sdp_conn_put(struct sdp_sock *conn)
+/*
+ * safe to call if there's another reference on connection.
+ */
+static inline void sdp_conn_put_light(struct sdp_sock *conn)
 {
-	if (atomic_dec_and_test(&conn->refcnt))
-		sdp_conn_destruct(conn);
+	int noref = atomic_dec_and_test(&conn->refcnt);
+	BUG_ON(noref);
 }
 
+void sdp_conn_put(struct sdp_sock *conn);
+
 static inline void *hashent_arg(s32 hashent) 
 {
 	return (void *)(unsigned long)hashent;

-- 
MST