[openib-general] [PATCH] Re: SDP_CONN_LOCK

Libor Michalek libor at topspin.com
Fri Feb 18 15:51:44 PST 2005


On Thu, Feb 17, 2005 at 03:49:31PM -0800, Sean Hefty wrote:
> Michael S. Tsirkin wrote:
> > Quoting r. Roland Dreier <roland at topspin.com>:
> >>
> >>BTW, since mthca currently calls completion handlers directly from
> >>interrupt context (rather than BH/tasklet context), it might be worth
> >>renaming all the SDP locking macros so they're not confusingly named
> >>with _BH suffixes.
> > 
> > I think it would be much nicer to reduce the number of macros used.
> 
> I think these would probably be better off as just function calls, 
> rather than macros.  SDP_CONN_LOCK calls sdp_conn_internal_lock(), and 
> that appears to be the only place that the function is called. 
> Similarly, SDP_CONN_UNLOCK calls sdp_conn_internal_unlock().  It seems 
> that you could just merge the macros into the function calls.

  Attached is a patch that replaces some of the connection macros with
inline function. The two internal functions you mention were not removed.
When the lock has no contention, which is the common case, those functions
are never called which is why the code was split into a macro and funciton
in the first place. The BH suffixes were replaced w/ IRQ.
  
Replaced macros:

  SDP_CONN_LOCK
  SDP_CONN_UNLOCK
  SDP_CONN_RELOCK
  SDP_CONN_HOLD
  SDP_CONN_PUT
  SDP_CONN_ERROR

Signed-off-by: Libor Michalek <libor at topspin.com>

Index: sdp_inet.c
===================================================================
--- sdp_inet.c	(revision 1836)
+++ sdp_inet.c	(working copy)
@@ -375,7 +375,7 @@
 	 */
 	sock->sk = NULL;
 
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 	conn->shutdown = SHUTDOWN_MASK;
 
 	if (SDP_SOCK_ST_LISTEN == conn->istate) {
@@ -456,9 +456,9 @@
 			while (0 < timeout &&
 			       0 == (SDP_ST_MASK_CLOSED & conn->istate)) {
 
-				SDP_CONN_UNLOCK(conn);
+				sdp_conn_unlock(conn);
 				timeout = schedule_timeout(timeout);
-				SDP_CONN_LOCK(conn);
+				sdp_conn_lock(conn);
 
 				if (signal_pending(current)) {
 					
@@ -499,8 +499,8 @@
 	 * finally drop socket reference. (socket API reference)
 	 */
 	sock_orphan(sk);
-	SDP_CONN_UNLOCK(conn);
-	SDP_CONN_PUT(conn);
+	sdp_conn_unlock(conn);
+	sdp_conn_put(conn);
 
 	return 0;
 } /* _sdp_inet_release */
@@ -568,7 +568,7 @@
 	/*
 	 * socket checks.
 	 */
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	if (SDP_SOCK_ST_CLOSED != conn->istate || 0 < conn->src_port) {
 
@@ -613,7 +613,7 @@
 
 	result = 0;
 done:
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 	return result;
 } /* _sdp_inet_bind */
 
@@ -663,7 +663,7 @@
 	/*
 	 * lock socket
 	 */
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	switch (sock->state) {
 	case SS_UNCONNECTED:
@@ -696,7 +696,7 @@
 		conn->dst_addr = ntohl(addr->sin_addr.s_addr);
 		conn->dst_port = ntohs(addr->sin_port);
 
-		SDP_CONN_HOLD(conn);	/* CM reference */
+		sdp_conn_hold(conn);	/* CM reference */
 		/*
 		 * close, allow connection completion notification.
 		 */
@@ -716,7 +716,7 @@
 			sock->state = SS_UNCONNECTED;
 			conn->istate = SDP_SOCK_ST_CLOSED;
 
-			SDP_CONN_PUT(conn);	/* CM reference */
+			sdp_conn_put(conn);	/* CM reference */
 
 			goto done;
 		}
@@ -751,9 +751,9 @@
 
 		while (0 < timeout && SDP_SOCK_ST_CONNECT == conn->istate) {
 
-			SDP_CONN_UNLOCK(conn);
+			sdp_conn_unlock(conn);
 			timeout = schedule_timeout(timeout);
-			SDP_CONN_LOCK(conn);
+			sdp_conn_lock(conn);
 
 			if (signal_pending(current)) {
 
@@ -792,7 +792,7 @@
 	case SDP_SOCK_ST_CLOSED:
 	case SDP_SOCK_ST_ERROR:
 
-		result = SDP_CONN_ERROR(conn) ? : -ECONNABORTED;
+		result = sdp_conn_error(conn) ? : -ECONNABORTED;
 		sock->state = SS_UNCONNECTED;
 		break;
 	default:
@@ -805,7 +805,7 @@
 	sdp_dbg_ctrl(conn, "CONNECT complete");
 
 done:
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 	return result;
 } /* _sdp_inet_connect */
 
@@ -827,7 +827,7 @@
 	sdp_dbg_ctrl(conn, "LISTEN: addr <%08x:%04x> backlog <%04x>",
 		     conn->src_addr, conn->src_port, backlog);
 
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	if (SS_UNCONNECTED != sock->state ||
 	    (SDP_SOCK_ST_CLOSED != conn->istate &&
@@ -870,7 +870,7 @@
 	result = 0;
 
 done:
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 	return result;
 } /* _sdp_inet_listen */
 
@@ -898,7 +898,7 @@
 	sdp_dbg_ctrl(listen_conn, "ACCEPT: addr <%08x:%04x>",
 		     listen_conn->src_addr, listen_conn->src_port);
 
-	SDP_CONN_LOCK(listen_conn);
+	sdp_conn_lock(listen_conn);
 
 	if (SDP_SOCK_ST_LISTEN != listen_conn->istate) {
 
@@ -926,9 +926,9 @@
 			       SDP_SOCK_ST_LISTEN == listen_conn->istate &&
 			       0 == listen_conn->backlog_cnt) {
 
-				SDP_CONN_UNLOCK(listen_conn);
+				sdp_conn_unlock(listen_conn);
 				timeout = schedule_timeout(timeout);
-				SDP_CONN_LOCK(listen_conn);
+				sdp_conn_lock(listen_conn);
 
 				if (signal_pending(current)) {
 
@@ -1015,9 +1015,9 @@
 				 * trying.
 				 */
 				/* AcceptQueueGet */
-				SDP_CONN_UNLOCK(accept_conn);
+				sdp_conn_unlock(accept_conn);
 				/* INET reference (AcceptQueue ref) */
-				SDP_CONN_PUT(accept_conn);
+				sdp_conn_put(accept_conn);
 
 				accept_sk = NULL;
 				accept_sock->sk = NULL;
@@ -1034,14 +1034,14 @@
 				 * lock
 				 */
 				/* AcceptQueueGet */
-				SDP_CONN_UNLOCK(accept_conn);
+				sdp_conn_unlock(accept_conn);
 			}
 		}
 	}
 
 	result = 0;
 listen_done:
-	SDP_CONN_UNLOCK(listen_conn);
+	sdp_conn_unlock(listen_conn);
 
 	sdp_dbg_ctrl(listen_conn, 
 		     "ACCEPT: complete <%d> <%08x:%04x><%08x:%04x>",
@@ -1287,7 +1287,7 @@
 		 */
 	case SIOCINQ:
 
-		SDP_CONN_LOCK(conn);
+		sdp_conn_lock(conn);
 
 		if (SDP_SOCK_ST_LISTEN != conn->istate) {
 			/*
@@ -1301,11 +1301,11 @@
 			result = -EINVAL;
 		}
 
-		SDP_CONN_UNLOCK(conn);
+		sdp_conn_unlock(conn);
 		break;
 	case SIOCOUTQ:
 
-		SDP_CONN_LOCK(conn);
+		sdp_conn_lock(conn);
 
 		if (SDP_SOCK_ST_LISTEN != conn->istate) {
 
@@ -1317,11 +1317,11 @@
 			result = -EINVAL;
 		}
 
-		SDP_CONN_UNLOCK(conn);
+		sdp_conn_unlock(conn);
 		break;
 	case SIOCATMARK:
 
-		SDP_CONN_LOCK(conn);
+		sdp_conn_lock(conn);
 
 		value = 0;
 
@@ -1338,7 +1338,7 @@
 
 		result = put_user(value, (int __user *) arg);
 
-		SDP_CONN_UNLOCK(conn);
+		sdp_conn_unlock(conn);
 		break;
 	default:
 
@@ -1385,7 +1385,7 @@
 		return -EFAULT;
 	}
 
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	switch (optname) {
 	case TCP_NODELAY:
@@ -1431,7 +1431,7 @@
 		break;
 	}
 
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 	return result;
 } /* _sdp_inet_setopt */
 
@@ -1473,7 +1473,7 @@
 		return -EINVAL;
 	}
 
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	switch (optname) {
 	case TCP_NODELAY:
@@ -1504,7 +1504,7 @@
 		break;
 	}
 
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 
 	if (put_user(len, optlen)) {
 
@@ -1546,7 +1546,7 @@
 		flag++;		/* match shutdown mask. */
 	}
 
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	conn->shutdown |= flag;
 
@@ -1572,7 +1572,7 @@
 		break;
 	case SDP_SOCK_ST_ERROR:
 
-		result = SDP_CONN_ERROR(conn);
+		result = sdp_conn_error(conn);
 		result = (result < 0) ? result : -ECONNABORTED;
 		break;
 	case SDP_SOCK_ST_ACCEPTED:
@@ -1629,7 +1629,7 @@
 		sdp_inet_wake_error(sock->sk);
 	}
 
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 	return result;
 } /* _sdp_inet_shutdown */
 
Index: sdp_proto.h
===================================================================
--- sdp_proto.h	(revision 1836)
+++ sdp_proto.h	(working copy)
@@ -44,7 +44,6 @@
 #include "sdp_advt.h"
 #include "sdp_iocb.h"
 #include "sdp_queue.h"
-
 /*
  * Buffer managment
  */
@@ -294,8 +293,6 @@
 		      u8 hw_port,
 		      u16 pkey);
 
-int sdp_conn_destruct(struct sdp_opt *conn);
-
 void sdp_inet_wake_send(struct sock *sk);
 
 void sdp_inet_wake_generic(struct sock *sk);
@@ -412,17 +409,6 @@
 int sdp_event_write(struct sdp_opt *conn, struct ib_wc *comp);
 
 /*
- * internal connection lock functions
- */
-void sdp_conn_internal_lock(struct sdp_opt *conn, unsigned long *flags);
-
-void sdp_conn_internal_unlock(struct sdp_opt *conn);
-
-void sdp_conn_internal_relock(struct sdp_opt *conn);
-
-int sdp_conn_cq_drain(struct ib_cq *cq, struct sdp_opt *conn);
-
-/*
  * DATA transport
  */
 int sdp_inet_send(struct kiocb *iocb,
@@ -686,39 +672,4 @@
 	return 0;
 } /* __sdp_conn_state_dump */
 
-/*
- * __sdp_conn_hold - increment reference count
- */
-static inline void __sdp_conn_hold(struct sdp_opt *conn)
-{
-	atomic_inc(&conn->refcnt);
-} /* __sdp_conn_hold */
-
-/*
- * __sdp_conn_put - decrement reference count
- */
-static inline void __sdp_conn_put(struct sdp_opt *conn)
-{
-	if (atomic_dec_and_test(&conn->refcnt)) {
-
-		(void)sdp_conn_destruct(conn);
-	}
-} /* __sdp_conn_hold */
-
-/*
- * __sdp_conn_error - get the connections error value destructively
- */
-static inline int __sdp_conn_error(struct sdp_opt *conn)
-{
-	/*
-	 * The connection error parameter is set and read under the connection
-	 * lock, however the linux socket error, needs to be xchg'd since the
-	 * SO_ERROR getsockopt happens outside of the connection lock.
-	 */
-	int error = xchg(&conn->sk->sk_err, 0);
-	SDP_CONN_SET_ERR(conn, 0);
-
-	return -error;
-} /* __sdp_conn_error */
-
 #endif /* _SDP_PROTO_H */
Index: sdp_send.c
===================================================================
--- sdp_send.c	(revision 1836)
+++ sdp_send.c	(working copy)
@@ -2044,7 +2044,7 @@
 	 * lock the socket while we operate.
 	 */
 	conn = SDP_GET_CONN(si->sock->sk);
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	sdp_dbg_ctrl(conn, "Cancel Write IOCB. <%08x:%04x> <%08x:%04x>",
 		     conn->src_addr, conn->src_port,
@@ -2168,7 +2168,7 @@
 	result = -EAGAIN;
 
 unlock:
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 done:
 
 	aio_put_req(req); /* cancel call reference */
@@ -2315,7 +2315,7 @@
 		     req->ki_key, msg->msg_iov->iov_base,
 		     req->ki_users, req->ki_flags);
 	
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 	/*
 	 * ESTABLISED and CLOSE can send, while CONNECT and ACCEPTED can
 	 * continue being processed, it'll wait below until the send window
@@ -2416,7 +2416,7 @@
 
 		if (0 != SDP_CONN_GET_ERR(conn)) {
 
-			result = (0 < copied) ? 0 : SDP_CONN_ERROR(conn);
+			result = (0 < copied) ? 0 : sdp_conn_error(conn);
 			break;
 		}
 
@@ -2464,12 +2464,12 @@
 			 */
 			clear_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
 
-			SDP_CONN_UNLOCK(conn);
+			sdp_conn_unlock(conn);
 			if (!(0 < __sdp_inet_write_space(conn, oob))) {
 
 				timeout = schedule_timeout(timeout);
 			}
-			SDP_CONN_LOCK(conn);
+			sdp_conn_lock(conn);
 
 			remove_wait_queue(sk->sk_sleep, &wait);
 			set_current_state(TASK_RUNNING);
@@ -2526,7 +2526,7 @@
 	}
 
 done:
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 	result = ((0 < copied) ? copied : result);
 
 	if (-EPIPE == result && 0 == (MSG_NOSIGNAL & msg->msg_flags)) {
Index: sdp_actv.c
===================================================================
--- sdp_actv.c	(revision 1836)
+++ sdp_actv.c	(working copy)
@@ -194,7 +194,7 @@
 drop:
 	sdp_inet_wake_error(sk);
 
-	SDP_CONN_PUT(conn);	/* CM sk reference */
+	sdp_conn_put(conn);	/* CM sk reference */
 	return result;
 } /* _sdp_actv_conn_establish */
 
@@ -295,7 +295,7 @@
 			goto done;
 		}
 
-		SDP_CONN_PUT(conn);
+		sdp_conn_put(conn);
 
 		break;
 	case SDP_CONN_ST_REQ_SENT:
@@ -389,7 +389,7 @@
 	/*
 	 * lock the socket
 	 */
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 	/*
 	 * path lookup is complete
 	 */
@@ -570,8 +570,8 @@
 	}
 	/* if */
 done:
-	SDP_CONN_UNLOCK(conn);
-	SDP_CONN_PUT(conn);
+	sdp_conn_unlock(conn);
+	sdp_conn_put(conn);
 
 	return;
 } /* _sdp_cm_path_complete */
@@ -599,8 +599,8 @@
 	/*
 	 * lookup the remote address
 	 */
-	SDP_CONN_HOLD(conn);
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_hold(conn);
+	sdp_conn_unlock(conn);
 					  
 	result = sdp_link_path_lookup(htonl(conn->dst_addr),
 				      htonl(conn->src_addr),
@@ -608,7 +608,7 @@
 				      _sdp_cm_path_complete,
 				      conn,
 				      &conn->plid);
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	if (0 > result) {
 
@@ -619,7 +619,7 @@
 		/*
 		 * callback dosn't have this socket.
 		 */
-		SDP_CONN_PUT(conn);
+		sdp_conn_put(conn);
 
 		result = -EDESTADDRREQ;
 		goto error;
Index: sdp_conn.c
===================================================================
--- sdp_conn.c	(revision 1836)
+++ sdp_conn.c	(working copy)
@@ -104,7 +104,7 @@
 	/*
 	 * up ref until we release. One ref for GW and one for INET.
 	 */
-	SDP_CONN_HOLD(accept_conn);	/* INET reference */
+	sdp_conn_hold(accept_conn);	/* INET reference */
 
 	return 0;
 } /* sdp_inet_accept_q_put */
@@ -136,7 +136,7 @@
 	 */
 	accept_conn = listen_conn->accept_prev;
 
-	SDP_CONN_LOCK(accept_conn);
+	sdp_conn_lock(accept_conn);
 
 	prev_conn = accept_conn->accept_prev;
 
@@ -172,7 +172,7 @@
 	 * list. The process context lock is used, so the function may
 	 * not be called from the CQ interrupt.
 	 */
-	SDP_CONN_LOCK(accept_conn->parent);
+	sdp_conn_lock(accept_conn->parent);
 
 	next_conn = accept_conn->accept_next;
 	prev_conn = accept_conn->accept_prev;
@@ -182,7 +182,7 @@
 
 	accept_conn->parent->backlog_cnt--;
 
-	SDP_CONN_UNLOCK(accept_conn->parent);
+	sdp_conn_unlock(accept_conn->parent);
 
 	accept_conn->accept_next = NULL;
 	accept_conn->accept_prev = NULL;
@@ -284,9 +284,9 @@
 		}
 
 		/* AcceptQueueGet */
-		SDP_CONN_UNLOCK(accept_conn);
+		sdp_conn_unlock(accept_conn);
 		/* INET reference (AcceptQueuePut). */
-		SDP_CONN_PUT(accept_conn);
+		sdp_conn_put(accept_conn);
 	}
 
 	listen_conn->accept_next = NULL;
@@ -315,7 +315,7 @@
 		if (port == conn->src_port &&
 		    (INADDR_ANY == conn->src_addr || addr == conn->src_addr)) {
 
-			SDP_CONN_HOLD(conn);
+			sdp_conn_hold(conn);
 			break;
 		}
 	}
@@ -661,7 +661,7 @@
 		goto done;
 	}
 
-	SDP_CONN_HOLD(conn);
+	sdp_conn_hold(conn);
 done:
 	spin_unlock_irqrestore(&_dev_root_s.sock_lock, flags);
 	return conn;
@@ -857,16 +857,19 @@
 } /* sdp_conn_internal_lock */
 
 /*
- * sdp_conn_internal_relock - test the connection (use only from macro)
+ * sdp_conn_relock - test the connection (use only from macro)
  */
-void sdp_conn_internal_relock(struct sdp_opt *conn)
+void sdp_conn_relock(struct sdp_opt *conn)
 {
+	unsigned long flags;
 	struct ib_wc entry;
 	int result_r;
 	int result_s;
 	int result;
 	int rearm = 1;
 
+	spin_lock_irqsave(&conn->lock.slock, flags);
+
 	while (1) {
 
 		result_r = ib_poll_cq(conn->recv_cq, 1, &entry);
@@ -934,8 +937,9 @@
 
 	conn->flags &= ~SDP_CONN_F_MASK_EVENT;
 
+	spin_unlock_irqrestore(&conn->lock.slock, flags);
 	return;
-} /* sdp_conn_internal_relock */
+} /* sdp_conn_relock */
 
 /*
  * sdp_conn_cq_drain - drain one of the the connection's CQs
Index: sdp_recv.c
===================================================================
--- sdp_recv.c	(revision 1836)
+++ sdp_recv.c	(working copy)
@@ -1070,7 +1070,7 @@
 	 * lock the socket while we operate.
 	 */
 	conn = SDP_GET_CONN(si->sock->sk);
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	sdp_dbg_ctrl(conn, "Cancel Read IOCB. <%08x:%04x> <%08x:%04x>",
 		     conn->src_addr, conn->src_port,
@@ -1178,7 +1178,7 @@
 	result = -EAGAIN;
 	
 unlock:
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 done:
 	aio_put_req(req);
 	return result;
@@ -1364,7 +1364,7 @@
 		msg->msg_flags |= MSG_PEEK;
 	}
 
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 
 	if (SDP_SOCK_ST_LISTEN == conn->istate ||
 	    SDP_SOCK_ST_CLOSED == conn->istate) {
@@ -1548,7 +1548,7 @@
 			/*
 			 * process backlog
 			 */
-			SDP_CONN_RELOCK(conn);
+			sdp_conn_relock(conn);
 
 			if (0 < sdp_buff_q_size(&conn->recv_pool)) {
 
@@ -1589,7 +1589,7 @@
 		 */
 		if (0 != SDP_CONN_GET_ERR(conn)) {
 
-			result = (0 < copied) ? 0 : SDP_CONN_ERROR(conn);
+			result = (0 < copied) ? 0 : sdp_conn_error(conn);
 			break;
 		}
 		
@@ -1625,9 +1625,9 @@
 
 			if (0 == sdp_buff_q_size(&conn->recv_pool)) {
 
-				SDP_CONN_UNLOCK(conn);
+				sdp_conn_unlock(conn);
 				timeout = schedule_timeout(timeout);
-				SDP_CONN_LOCK(conn);
+				sdp_conn_lock(conn);
 			}
 
 			clear_bit(SOCK_ASYNC_WAITDATA, &sk->sk_socket->flags);
@@ -1725,6 +1725,6 @@
 		}
 	}
 
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 	return ((0 < copied) ? copied : result);
 } /* sdp_inet_recv */
Index: sdp_wall.c
===================================================================
--- sdp_wall.c	(revision 1836)
+++ sdp_wall.c	(working copy)
@@ -329,7 +329,7 @@
 
 	sdp_inet_wake_error(conn->sk);
 
-	SDP_CONN_PUT(conn);	/* CM reference */
+	sdp_conn_put(conn);	/* CM reference */
 
 	return 0;
 } /* sdp_wall_recv_reject */
@@ -402,7 +402,7 @@
 
 	sdp_inet_wake_error(conn->sk);
 	
-	SDP_CONN_PUT(conn);	/* CM sk reference */
+	sdp_conn_put(conn);	/* CM sk reference */
 	
 	return result;
 } /* sdp_wall_recv_confirm */
@@ -439,7 +439,7 @@
 		break;
 	}
 
-	SDP_CONN_PUT(conn);	/* CM reference */
+	sdp_conn_put(conn);	/* CM reference */
 
 	return 0;
 } /* sdp_wall_recv_failed */
@@ -651,7 +651,7 @@
 
 	conn->send_buf = 0;
 
-	SDP_CONN_PUT(conn);	/* CM sk reference */
+	sdp_conn_put(conn);	/* CM sk reference */
 
 	return 0;
 } /* sdp_wall_recv_drop */
Index: sdp_conn.h
===================================================================
--- sdp_conn.h	(revision 1836)
+++ sdp_conn.h	(working copy)
@@ -58,8 +58,8 @@
 #define SDP_CONN_F_SEND_CQ_PEND 0x0080 /* send CQ event is pending */
 #define SDP_CONN_F_DEAD         0xFFFF /* connection has been deleted */
 
-#define SDP_CONN_F_MASK_EVENT   (SDP_CONN_F_RECV_CQ_PEND | \
-				    SDP_CONN_F_SEND_CQ_PEND)
+#define SDP_CONN_F_MASK_EVENT   (SDP_CONN_F_RECV_CQ_PEND| \
+				 SDP_CONN_F_SEND_CQ_PEND)
 /*
  * SDP states.
  */
@@ -461,47 +461,80 @@
 /*
  * SDP connection lock
  */
-#define SDP_CONN_LOCK_BH(conn) \
+extern void sdp_conn_internal_lock(struct sdp_opt *conn, unsigned long *flags);
+extern void sdp_conn_internal_unlock(struct sdp_opt *conn);
+extern void sdp_conn_relock(struct sdp_opt *conn);
+extern int sdp_conn_cq_drain(struct ib_cq *cq, struct sdp_opt *conn);
+extern int sdp_conn_destruct(struct sdp_opt *conn);
+
+#define SDP_CONN_LOCK_IRQ(conn, flags) \
         spin_lock_irqsave(&((conn)->lock.slock), flags)
-#define SDP_CONN_UNLOCK_BH(conn) \
+#define SDP_CONN_UNLOCK_IRQ(conn, flags) \
         spin_unlock_irqrestore(&((conn)->lock.slock), flags)
 
-#define SDP_CONN_LOCK(conn)                             \
-do {                                                    \
-  unsigned long flags;                                  \
-  spin_lock_irqsave(&((conn)->lock.slock), flags);      \
-  if ((conn)->lock.users != 0) {                        \
-     sdp_conn_internal_lock(conn, &flags);              \
-  } /* if */                                            \
-  (conn)->lock.users = 1;                               \
-  spin_unlock_irqrestore(&((conn)->lock.slock), flags); \
-} while(0)
+static inline void sdp_conn_lock(struct sdp_opt *conn)
+{
+	unsigned long flags;
 
-#define SDP_CONN_UNLOCK(conn)                           \
-do {                                                    \
-  unsigned long flags;                                  \
-  spin_lock_irqsave(&((conn)->lock.slock), flags);      \
-  if (0 < (SDP_CONN_F_MASK_EVENT & conn->flags) &&   \
-      0 < (SDP_ST_MASK_EVENTS & conn->state)) {      \
-     sdp_conn_internal_unlock(conn);                    \
-  } /* if */                                            \
-  (conn)->lock.users = 0;                               \
-  wake_up(&((conn)->lock.waitq));                       \
-  spin_unlock_irqrestore(&((conn)->lock.slock), flags); \
-} while(0)
+	spin_lock_irqsave(&conn->lock.slock, flags);
+	if (conn->lock.users != 0) {
 
-#define SDP_CONN_RELOCK(conn)                           \
-do {                                                    \
-  unsigned long flags;                                  \
-  spin_lock_irqsave(&((conn)->lock.slock), flags);      \
-     sdp_conn_internal_relock(conn);                    \
-  spin_unlock_irqrestore(&((conn)->lock.slock), flags); \
-} while(0)
+		sdp_conn_internal_lock(conn, &flags);
+	}
 
-#define SDP_CONN_HOLD(conn) __sdp_conn_hold((conn))
-#define SDP_CONN_PUT(conn)  __sdp_conn_put((conn))
-#define SDP_CONN_ERROR(conn)  __sdp_conn_error((conn))
+	conn->lock.users = 1;
+	spin_unlock_irqrestore(&(conn->lock.slock), flags);
+}
 
+static inline void sdp_conn_unlock(struct sdp_opt *conn)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&conn->lock.slock, flags);
+	if (0 < (SDP_CONN_F_MASK_EVENT & conn->flags) &&
+	    0 < (SDP_ST_MASK_EVENTS & conn->state)) {
+
+		sdp_conn_internal_unlock(conn);
+	}
+
+	conn->lock.users = 0;
+	wake_up(&conn->lock.waitq);
+
+	spin_unlock_irqrestore(&conn->lock.slock, flags);
+}
+
+/*
+ * connection reference counting.
+ */
+static inline void sdp_conn_hold(struct sdp_opt *conn)
+{
+	atomic_inc(&conn->refcnt);
+}
+
+static inline void sdp_conn_put(struct sdp_opt *conn)
+{
+	if (atomic_dec_and_test(&conn->refcnt)) {
+
+		(void)sdp_conn_destruct(conn);
+	}
+}
+
+/*
+ * sdp_conn_error - get the connections error value destructively
+ */
+static inline int sdp_conn_error(struct sdp_opt *conn)
+{
+	/*
+	 * The connection error parameter is set and read under the connection
+	 * lock, however the linux socket error, needs to be xchg'd since the
+	 * SO_ERROR getsockopt happens outside of the connection lock.
+	 */
+	int error = xchg(&conn->sk->sk_err, 0);
+	SDP_CONN_SET_ERR(conn, 0);
+
+	return -error;
+}
+
 static inline void *__hashent_arg(s32 hashent) 
 {
 	return (void *)(unsigned long)hashent;
Index: sdp_pass.c
===================================================================
--- sdp_pass.c	(revision 1836)
+++ sdp_pass.c	(working copy)
@@ -218,7 +218,7 @@
 		goto lookup_err;
 	}
 
-	SDP_CONN_LOCK(listen_conn);
+	sdp_conn_lock(listen_conn);
 	/*
 	 * check backlog
 	 */
@@ -304,8 +304,8 @@
 
 	result = 0;
 locked_err:
-	SDP_CONN_UNLOCK(listen_conn);
-	SDP_CONN_PUT(listen_conn);	/* ListenLookup reference. */
+	sdp_conn_unlock(listen_conn);
+	sdp_conn_put(listen_conn);	/* ListenLookup reference. */
 lookup_err:
 	return result;
 } /* _sdp_cm_listen_lookup */
@@ -421,7 +421,7 @@
 	/*
 	 * Lock the new connection before modifyingg it into any tables.
 	 */
-	SDP_CONN_LOCK(conn);
+	sdp_conn_lock(conn);
 	/*
 	 * save hello parameters.
 	 */
@@ -488,12 +488,12 @@
 	/*
 	 * unlock
 	 */
-	SDP_CONN_UNLOCK(conn);
+	sdp_conn_unlock(conn);
 	return 0;
 error:
 	conn->cm_id = NULL; /* cm_id destroyed by CM on error result. */
-	SDP_CONN_UNLOCK(conn);
-	SDP_CONN_PUT(conn);	/* CM sk reference, will destory */
+	sdp_conn_unlock(conn);
+	sdp_conn_put(conn);	/* CM sk reference, will destory */
 done:
 	return result;
 } /* sdp_cm_req_handler */
Index: sdp_event.c
===================================================================
--- sdp_event.c	(revision 1836)
+++ sdp_event.c	(working copy)
@@ -162,7 +162,7 @@
 	 * lock the bottom half of the socket. If the connection is in use,
 	 * then queue the event, otherwise process this event.
 	 */
-	SDP_CONN_LOCK_BH(conn);
+	SDP_CONN_LOCK_IRQ(conn, flags);
 	/*
 	 * Check for event completions before CM has transitioned to
 	 * the established state. The CQ will not be polled or rearmed
@@ -206,8 +206,8 @@
 	}
 
 unlock:
-	SDP_CONN_UNLOCK_BH(conn);
-	SDP_CONN_PUT(conn);
+	SDP_CONN_UNLOCK_IRQ(conn, flags);
+	sdp_conn_put(conn);
 done:
 	return;
 } /* sdp_cq_event_handler */
@@ -553,7 +553,7 @@
 	conn = sdp_conn_table_lookup(hashent);
 	if (NULL != conn) {
 
-		SDP_CONN_LOCK(conn);
+		sdp_conn_lock(conn);
 	}
 	else {
 
@@ -608,8 +608,8 @@
 			(void)__sdp_conn_state_dump(conn);
 		}
 
-		SDP_CONN_UNLOCK(conn);
-		SDP_CONN_PUT(conn);
+		sdp_conn_unlock(conn);
+		sdp_conn_put(conn);
 	}
 
 	return result;
Index: sdp_post.c
===================================================================
--- sdp_post.c	(revision 1836)
+++ sdp_post.c	(working copy)
@@ -124,7 +124,7 @@
 		sdp_dbg_warn(NULL, "Error <%d> CM disconnect request", result);
 	}
 
-	SDP_CONN_PUT(conn);
+	sdp_conn_put(conn);
 
 	return;
 } /* _sdp_cm_disconnect */
@@ -154,7 +154,7 @@
 		sdp_dbg_warn(NULL, "Error <%d> CM reject request", result);
 	}
 
-	SDP_CONN_PUT(conn);
+	sdp_conn_put(conn);
 
 	return;
 } /* _sdp_cm_reject */
@@ -182,7 +182,7 @@
 		sdp_dbg_warn(NULL, "Error <%d> CM confirm request", result);
 	}
 
-	SDP_CONN_PUT(conn);
+	sdp_conn_put(conn);
 
 	return;
 } /* _sdp_cm_confirm */
@@ -213,7 +213,7 @@
 		sdp_dbg_warn(NULL, "Error <%d> CM failed request", result);
 	}
 
-	SDP_CONN_PUT(conn);
+	sdp_conn_put(conn);
 
 	return;
 } /* _sdp_cm_failed */
@@ -228,7 +228,7 @@
 	/*
 	 * send a potentially defered failed request.
 	 */
-	SDP_CONN_HOLD(conn);
+	sdp_conn_hold(conn);
 
 	if (in_atomic() || irqs_disabled()) {
 




More information about the general mailing list