PERFORCE change 43702 for review

[Sam Leffler]

http://perforce.freebsd.org/chv.cgi?CH=43702

Change 43702 by sam at sam_ebb on 2003/12/09 15:13:49

	introduce COMMON_START/COMMON_END mechanism a la tcp to
	unify lock handling; add socket lock reordering to satisfy
	output path locking requirements

Affected files ...

.. //depot/projects/netperf+sockets/sys/netinet/udp_usrreq.c#3 edit

Differences ...

==== //depot/projects/netperf+sockets/sys/netinet/udp_usrreq.c#3 (text+ko) ====

@@ -904,24 +904,67 @@
 SYSCTL_INT(_net_inet_udp, UDPCTL_RECVSPACE, recvspace, CTLFLAG_RW,
     &udp_recvspace, 0, "Maximum incoming UDP datagram size");
 
+/*
+ * Common code to setup and teardown locking.  Most
+ * code begins with a COMMON_START macro and finishes
+ * with COMMON_END.  You indicate whether the inpcb
+ * and enclosing head are to be locked read or write 
+ * and whether there is an existing sockbuf lock that
+ * needs to be re-ordered.
+ */
+#define INI_NOLOCK	0		/* no head lock */
+#define INI_READ	1		/* read head lock */
+#define INI_WRITE	2		/* write head lock */
+#define SBI_NONE	0		/* no sockbuf lock to reorder */
+#define SBI_SND		1		/* reorder so->so_snd lock */
+#define SBI_RCV		2		/* reorder so->so_rcv lock */
+
+#define	COMMON_START(_headrw, _sbrw) do {			\
+	if (_sbrw == SBI_SND)					\
+		SOCKBUF_UNLOCK(&so->so_snd);			\
+	else if (_sbrw == SBI_RCV)				\
+		SOCKBUF_UNLOCK(&so->so_rcv);			\
+	if (_headrw == INI_READ)				\
+		INP_INFO_RLOCK(&udbinfo);			\
+	else if (_headrw == INI_WRITE)				\
+		INP_INFO_WLOCK(&udbinfo);			\
+	inp = sotoinpcb(so);					\
+	if (inp == 0) {						\
+		if (_sbrw == SBI_SND)				\
+			SOCKBUF_LOCK(&so->so_snd);		\
+		else if (_sbrw == SBI_RCV)			\
+			SOCKBUF_LOCK(&so->so_rcv);		\
+		if (_headrw == INI_READ)			\
+			INP_INFO_RUNLOCK(&udbinfo);		\
+		else if (_headrw == INI_WRITE)			\
+			INP_INFO_WUNLOCK(&udbinfo);		\
+		return EINVAL;					\
+	}							\
+	INP_LOCK(inp);						\
+	if (_sbrw == SBI_SND)					\
+		SOCKBUF_LOCK(&so->so_snd);			\
+	else if (_sbrw == SBI_RCV)				\
+		SOCKBUF_LOCK(&so->so_rcv);			\
+	if (_headrw == INI_READ)				\
+		INP_INFO_RUNLOCK(&udbinfo);			\
+} while(0)
+
+#define COMMON_END(_headrw)					\
+	do {							\
+		INP_UNLOCK(inp);				\
+		if (_headrw == INI_WRITE)			\
+			INP_INFO_WUNLOCK(&udbinfo);		\
+	} while(0)
+
 static int
 udp_abort(struct socket *so)
 {
 	struct inpcb *inp;
-	int s;
 
-	INP_INFO_WLOCK(&udbinfo);
-	inp = sotoinpcb(so);
-	if (inp == 0) {
-		INP_INFO_WUNLOCK(&udbinfo);
-		return EINVAL;	/* ??? possible? panic instead? */
-	}
-	INP_LOCK(inp);
+	COMMON_START(INI_WRITE, SBI_NONE);
 	soisdisconnected(so);
-	s = splnet();
 	in_pcbdetach(inp);
 	INP_INFO_WUNLOCK(&udbinfo);
-	splx(s);
 	return 0;
 }
 
@@ -963,20 +1006,11 @@
 udp_bind(struct socket *so, struct sockaddr *nam, struct thread *td)
 {
 	struct inpcb *inp;
-	int s, error;
+	int error;
 
-	INP_INFO_WLOCK(&udbinfo);
-	inp = sotoinpcb(so);
-	if (inp == 0) {
-		INP_INFO_WUNLOCK(&udbinfo);
-		return EINVAL;
-	}
-	INP_LOCK(inp);
-	s = splnet();
+	COMMON_START(INI_WRITE, SBI_NONE);
 	error = in_pcbbind(inp, nam, td);
-	splx(s);
-	INP_UNLOCK(inp);
-	INP_INFO_WUNLOCK(&udbinfo);
+	COMMON_END(INI_WRITE);
 	return error;
 }
 
@@ -984,31 +1018,22 @@
 udp_connect(struct socket *so, struct sockaddr *nam, struct thread *td)
 {
 	struct inpcb *inp;
-	int s, error;
+	int error;
 	struct sockaddr_in *sin;
 
-	INP_INFO_WLOCK(&udbinfo);
-	inp = sotoinpcb(so);
-	if (inp == 0) {
-		INP_INFO_WUNLOCK(&udbinfo);
-		return EINVAL;
-	}
-	INP_LOCK(inp);
+	COMMON_START(INI_WRITE, SBI_NONE);
 	if (inp->inp_faddr.s_addr != INADDR_ANY) {
-		INP_UNLOCK(inp);
-		INP_INFO_WUNLOCK(&udbinfo);
-		return EISCONN;
+		error = EISCONN;
+		goto out;
 	}
-	s = splnet();
 	sin = (struct sockaddr_in *)nam;
 	if (td && jailed(td->td_ucred))
 		prison_remote_ip(td->td_ucred, 0, &sin->sin_addr.s_addr);
 	error = in_pcbconnect(inp, nam, td);
-	splx(s);
 	if (error == 0)
 		soisconnected(so);
-	INP_UNLOCK(inp);
-	INP_INFO_WUNLOCK(&udbinfo);
+out:
+	COMMON_END(INI_WRITE);
 	return error;
 }
 
@@ -1016,49 +1041,31 @@
 udp_detach(struct socket *so)
 {
 	struct inpcb *inp;
-	int s;
 
-	INP_INFO_WLOCK(&udbinfo);
-	inp = sotoinpcb(so);
-	if (inp == 0) {
-		INP_INFO_WUNLOCK(&udbinfo);
-		return EINVAL;
-	}
-	INP_LOCK(inp);
-	s = splnet();
+	COMMON_START(INI_WRITE, SBI_NONE);
 	in_pcbdetach(inp);
 	INP_INFO_WUNLOCK(&udbinfo);
-	splx(s);
 	return 0;
 }
 
 static int
 udp_disconnect(struct socket *so)
 {
+	int error = 0;
 	struct inpcb *inp;
-	int s;
 
-	INP_INFO_WLOCK(&udbinfo);
-	inp = sotoinpcb(so);
-	if (inp == 0) {
-		INP_INFO_WUNLOCK(&udbinfo);
-		return EINVAL;
-	}
-	INP_LOCK(inp);
+	COMMON_START(INI_WRITE, SBI_NONE);
 	if (inp->inp_faddr.s_addr == INADDR_ANY) {
-		INP_INFO_WUNLOCK(&udbinfo);
-		INP_UNLOCK(inp);
-		return ENOTCONN;
+		error = ENOTCONN;
+		goto out;
 	}
 
-	s = splnet();
 	in_pcbdisconnect(inp);
 	inp->inp_laddr.s_addr = INADDR_ANY;
-	INP_UNLOCK(inp);
-	INP_INFO_WUNLOCK(&udbinfo);
-	splx(s);
+	COMMON_END(INI_WRITE);
 	so->so_state &= ~SS_ISCONNECTED;		/* XXX */
-	return 0;
+out:
+	return error;
 }
 
 static int
@@ -1066,20 +1073,23 @@
 	    struct mbuf *control, struct thread *td)
 {
 	struct inpcb *inp;
-	int ret;
+	int error;
 
+	SOCKBUF_UNLOCK(&so->so_snd);
 	INP_INFO_WLOCK(&udbinfo);
 	inp = sotoinpcb(so);
 	if (inp == 0) {
+		SOCKBUF_LOCK(&so->so_snd);
 		INP_INFO_WUNLOCK(&udbinfo);
 		m_freem(m);
 		return EINVAL;
 	}
 	INP_LOCK(inp);
-	ret = udp_output(inp, m, addr, control, td);
+	SOCKBUF_LOCK(&so->so_snd);
+	error = udp_output(inp, m, addr, control, td);
 	INP_UNLOCK(inp);
 	INP_INFO_WUNLOCK(&udbinfo);
-	return ret; 
+	return error; 
 }
 
 int
@@ -1087,16 +1097,9 @@
 {
 	struct inpcb *inp;
 
-	INP_INFO_RLOCK(&udbinfo);
-	inp = sotoinpcb(so);
-	if (inp == 0) {
-		INP_INFO_RUNLOCK(&udbinfo);
-		return EINVAL;
-	}
-	INP_LOCK(inp);
-	INP_INFO_RUNLOCK(&udbinfo);
+	COMMON_START(INI_READ, SBI_NONE);
 	socantsendmore(so);
-	INP_UNLOCK(inp);
+	COMMON_END(INI_READ);
 	return 0;
 }