Date: Tue, 9 Dec 2003 15:13:42 -0800 (PST) From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 43701 for review Message-ID: <200312092313.hB9NDgwM034602@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=43701 Change 43701 by sam@sam_ebb on 2003/12/09 15:12:54 redo COMMON_START/COMMON_END stuff to eliminate implicit use of labels and some local variables; add sockbuf lock reordering so satisfy inp head/inp/sockbuf lock ordering requirements Affected files ... .. //depot/projects/netperf+sockets/sys/netinet/tcp_usrreq.c#3 edit Differences ... ==== //depot/projects/netperf+sockets/sys/netinet/tcp_usrreq.c#3 (text+ko) ==== @@ -119,7 +119,6 @@ static int tcp_usr_attach(struct socket *so, int proto, struct thread *td) { - int s = splnet(); int error; struct inpcb *inp; struct tcpcb *tp = 0; @@ -145,11 +144,71 @@ out: TCPDEBUG2(PRU_ATTACH); INP_INFO_WUNLOCK(&tcbinfo); - splx(s); return error; } /* + * 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_START0(_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(&tcbinfo); \ + else if (_headrw == INI_WRITE) \ + INP_INFO_WLOCK(&tcbinfo); \ + 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(&tcbinfo); \ + else if (_headrw == INI_WRITE) \ + INP_INFO_WUNLOCK(&tcbinfo); \ + 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(&tcbinfo); \ + tp = intotcpcb(inp); \ + TCPDEBUG1(); \ +} while(0) + +#define COMMON_START(_headrw, _sbrw) do { \ + TCPDEBUG0; \ + COMMON_START0(_headrw, _sbrw); \ +} while (0) + +#define COMMON_END(_headrw, req) \ + TCPDEBUG2(req); \ + do { \ + if (tp) \ + INP_UNLOCK(inp); \ + if (_headrw == INI_WRITE) \ + INP_INFO_WUNLOCK(&tcbinfo); \ + } while(0) + +/* * pru_detach() detaches the TCP protocol from the socket. * If the protocol state is non-embryonic, then can't * do this directly: have to initiate a pru_disconnect(), @@ -159,85 +218,28 @@ static int tcp_usr_detach(struct socket *so) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - TCPDEBUG0; - INP_INFO_WLOCK(&tcbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_WUNLOCK(&tcbinfo); - splx(s); - return EINVAL; /* XXX */ - } - INP_LOCK(inp); - tp = intotcpcb(inp); - TCPDEBUG1(); + COMMON_START(INI_WRITE, SBI_NONE); tp = tcp_disconnect(tp); - - TCPDEBUG2(PRU_DETACH); - if (tp) - INP_UNLOCK(inp); - INP_INFO_WUNLOCK(&tcbinfo); - splx(s); + COMMON_END(INI_WRITE, PRU_DETACH); return error; } -#define INI_NOLOCK 0 -#define INI_READ 1 -#define INI_WRITE 2 - -#define COMMON_START() \ - TCPDEBUG0; \ - do { \ - if (inirw == INI_READ) \ - INP_INFO_RLOCK(&tcbinfo); \ - else if (inirw == INI_WRITE) \ - INP_INFO_WLOCK(&tcbinfo); \ - inp = sotoinpcb(so); \ - if (inp == 0) { \ - if (inirw == INI_READ) \ - INP_INFO_RUNLOCK(&tcbinfo); \ - else if (inirw == INI_WRITE) \ - INP_INFO_WUNLOCK(&tcbinfo); \ - splx(s); \ - return EINVAL; \ - } \ - INP_LOCK(inp); \ - if (inirw == INI_READ) \ - INP_INFO_RUNLOCK(&tcbinfo); \ - tp = intotcpcb(inp); \ - TCPDEBUG1(); \ -} while(0) - -#define COMMON_END(req) \ -out: TCPDEBUG2(req); \ - do { \ - if (tp) \ - INP_UNLOCK(inp); \ - if (inirw == INI_WRITE) \ - INP_INFO_WUNLOCK(&tcbinfo); \ - splx(s); \ - return error; \ - goto out; \ -} while(0) - /* * Give the socket an address. */ static int tcp_usr_bind(struct socket *so, struct sockaddr *nam, struct thread *td) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; struct sockaddr_in *sinp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); /* * Must check for multicast addresses and disallow binding @@ -250,23 +252,21 @@ goto out; } error = in_pcbbind(inp, nam, td); - if (error) - goto out; - COMMON_END(PRU_BIND); +out: + COMMON_END(INI_WRITE, PRU_BIND); + return error; } #ifdef INET6 static int tcp6_usr_bind(struct socket *so, struct sockaddr *nam, struct thread *td) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; struct sockaddr_in6 *sin6p; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); /* * Must check for multicast addresses and disallow binding @@ -294,9 +294,9 @@ } } error = in6_pcbbind(inp, nam, td); - if (error) - goto out; - COMMON_END(PRU_BIND); +out: + COMMON_END(INI_WRITE, PRU_BIND); + return error; } #endif /* INET6 */ @@ -306,31 +306,28 @@ static int tcp_usr_listen(struct socket *so, struct thread *td) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); if (inp->inp_lport == 0) error = in_pcbbind(inp, (struct sockaddr *)0, td); if (error == 0) tp->t_state = TCPS_LISTEN; - COMMON_END(PRU_LISTEN); + COMMON_END(INI_WRITE, PRU_LISTEN); + return error; } #ifdef INET6 static int tcp6_usr_listen(struct socket *so, struct thread *td) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); if (inp->inp_lport == 0) { inp->inp_vflag &= ~INP_IPV4; if ((inp->inp_flags & IN6P_IPV6_V6ONLY) == 0) @@ -339,7 +336,8 @@ } if (error == 0) tp->t_state = TCPS_LISTEN; - COMMON_END(PRU_LISTEN); + COMMON_END(INI_WRITE, PRU_LISTEN); + return error; } #endif /* INET6 */ @@ -353,14 +351,12 @@ static int tcp_usr_connect(struct socket *so, struct sockaddr *nam, struct thread *td) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; struct sockaddr_in *sinp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); /* * Must disallow TCP ``connections'' to multicast addresses. @@ -378,21 +374,21 @@ if ((error = tcp_connect(tp, nam, td)) != 0) goto out; error = tcp_output(tp); - COMMON_END(PRU_CONNECT); +out: + COMMON_END(INI_WRITE, PRU_CONNECT); + return error; } #ifdef INET6 static int tcp6_usr_connect(struct socket *so, struct sockaddr *nam, struct thread *td) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; struct sockaddr_in6 *sin6p; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); /* * Must disallow TCP ``connections'' to multicast addresses. @@ -426,7 +422,9 @@ if ((error = tcp6_connect(tp, nam, td)) != 0) goto out; error = tcp_output(tp); - COMMON_END(PRU_CONNECT); +out: + COMMON_END(INI_WRITE, PRU_CONNECT); + return error; } #endif /* INET6 */ @@ -444,15 +442,14 @@ static int tcp_usr_disconnect(struct socket *so) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); tp = tcp_disconnect(tp); - COMMON_END(PRU_DISCONNECT); + COMMON_END(INI_WRITE, PRU_DISCONNECT); + return error; } /* @@ -463,7 +460,6 @@ static int tcp_usr_accept(struct socket *so, struct sockaddr **nam) { - int s; int error = 0; struct inpcb *inp = NULL; struct tcpcb *tp = NULL; @@ -473,34 +469,21 @@ if (so->so_state & SS_ISDISCONNECTED) { error = ECONNABORTED; - goto out; + goto out; /* NB: ok 'cuz tp is NULL */ } - s = splnet(); - INP_INFO_RLOCK(&tcbinfo); - inp = sotoinpcb(so); - if (!inp) { - INP_INFO_RUNLOCK(&tcbinfo); - splx(s); - return (EINVAL); - } - INP_LOCK(inp); - INP_INFO_RUNLOCK(&tcbinfo); - tp = intotcpcb(inp); - TCPDEBUG1(); + COMMON_START0(INI_READ, SBI_NONE); /* - * We inline in_setpeeraddr and COMMON_END here, so that we can - * copy the data of interest and defer the malloc until after we - * release the lock. + * We inline in_setpeeraddr so that we can copy the + * data of interest and defer the malloc until after + * we release the lock. */ port = inp->inp_fport; addr = inp->inp_faddr; -out: TCPDEBUG2(PRU_ACCEPT); - if (tp) - INP_UNLOCK(inp); - splx(s); +out: + COMMON_END(INI_READ, PRU_ACCEPT); if (error == 0) *nam = in_sockaddr(port, &addr); return error; @@ -510,7 +493,6 @@ static int tcp6_usr_accept(struct socket *so, struct sockaddr **nam) { - int s; struct inpcb *inp = NULL; int error = 0; struct tcpcb *tp = NULL; @@ -522,25 +504,14 @@ if (so->so_state & SS_ISDISCONNECTED) { error = ECONNABORTED; - goto out; + goto out; /* NB: ok 'cuz tp is NULL */ } - s = splnet(); - INP_INFO_RLOCK(&tcbinfo); - inp = sotoinpcb(so); - if (inp == 0) { - INP_INFO_RUNLOCK(&tcbinfo); - splx(s); - return (EINVAL); - } - INP_LOCK(inp); - INP_INFO_RUNLOCK(&tcbinfo); - tp = intotcpcb(inp); - TCPDEBUG1(); + COMMON_START0(INI_READ, SBI_NONE); /* - * We inline in6_mapped_peeraddr and COMMON_END here, so that we can - * copy the data of interest and defer the malloc until after we - * release the lock. + * We inline in6_mapped_peeraddr so that we can + * copy the data of interest and defer the malloc + * until after we release the lock. */ if (inp->inp_vflag & INP_IPV4) { v4 = 1; @@ -551,10 +522,8 @@ addr6 = inp->in6p_faddr; } -out: TCPDEBUG2(PRU_ACCEPT); - if (tp) - INP_UNLOCK(inp); - splx(s); +out: + COMMON_END(INI_READ, PRU_ACCEPT); if (error == 0) { if (v4) *nam = in6_v4mapsin6_sockaddr(port, &addr); @@ -592,18 +561,17 @@ static int tcp_usr_shutdown(struct socket *so) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); socantsendmore(so); tp = tcp_usrclosed(tp); if (tp) error = tcp_output(tp); - COMMON_END(PRU_SHUTDOWN); + COMMON_END(INI_WRITE, PRU_SHUTDOWN); + return error; } /* @@ -612,15 +580,14 @@ static int tcp_usr_rcvd(struct socket *so, int flags) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_READ; - COMMON_START(); + COMMON_START(INI_READ, SBI_RCV); tcp_output(tp); - COMMON_END(PRU_RCVD); + COMMON_END(INI_READ, PRU_RCVD); + return error; } /* @@ -634,11 +601,9 @@ tcp_usr_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam, struct mbuf *control, struct thread *td) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; #ifdef INET6 int isipv6; #endif @@ -650,9 +615,11 @@ * We really want to have to this function upgrade from read lock * to write lock. XXX */ + SOCKBUF_UNLOCK(&so->so_snd); INP_INFO_WLOCK(&tcbinfo); inp = sotoinpcb(so); if (inp == NULL) { + SOCKBUF_LOCK(&so->so_snd); /* * OOPS! we lost a race, the TCP session got reset after * we checked SS_CANTSENDMORE, eg: while doing uiomove or a @@ -668,6 +635,7 @@ goto out; } INP_LOCK(inp); + SOCKBUF_LOCK(&so->so_snd); #ifdef INET6 isipv6 = nam && nam->sa_family == AF_INET6; #endif /* INET6 */ @@ -758,8 +726,10 @@ error = tcp_output(tp); tp->t_force = 0; } - COMMON_END((flags & PRUS_OOB) ? PRU_SENDOOB : +out: + COMMON_END(INI_WRITE, (flags & PRUS_OOB) ? PRU_SENDOOB : ((flags & PRUS_EOF) ? PRU_SEND_EOF : PRU_SEND)); + return error; } /* @@ -768,15 +738,14 @@ static int tcp_usr_abort(struct socket *so) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_WRITE; - COMMON_START(); + COMMON_START(INI_WRITE, SBI_NONE); tp = tcp_drop(tp, ECONNABORTED); - COMMON_END(PRU_ABORT); + COMMON_END(INI_WRITE, PRU_ABORT); + return error; } /* @@ -785,13 +754,11 @@ static int tcp_usr_rcvoob(struct socket *so, struct mbuf *m, int flags) { - int s = splnet(); int error = 0; struct inpcb *inp; struct tcpcb *tp; - const int inirw = INI_READ; - COMMON_START(); + COMMON_START(INI_READ, SBI_NONE); if ((so->so_oobmark == 0 && (so->so_state & SS_RCVATMARK) == 0) || so->so_options & SO_OOBINLINE || @@ -807,7 +774,9 @@ *mtod(m, caddr_t) = tp->t_iobc; if ((flags & MSG_PEEK) == 0) tp->t_oobflags ^= (TCPOOB_HAVEDATA | TCPOOB_HADDATA); - COMMON_END(PRU_RCVOOB); +out: + COMMON_END(INI_READ, PRU_RCVOOB); + return error; } /* xxx - should be const */ @@ -1026,17 +995,15 @@ struct socket *so; struct sockopt *sopt; { - int error, opt, optval, s; + int error, opt, optval; struct inpcb *inp; struct tcpcb *tp; error = 0; - s = splnet(); /* XXX */ INP_INFO_RLOCK(&tcbinfo); inp = sotoinpcb(so); if (inp == NULL) { INP_INFO_RUNLOCK(&tcbinfo); - splx(s); return (ECONNRESET); } INP_LOCK(inp); @@ -1049,7 +1016,6 @@ #endif /* INET6 */ error = ip_ctloutput(so, sopt); INP_UNLOCK(inp); - splx(s); return (error); } tp = intotcpcb(inp); @@ -1137,7 +1103,6 @@ break; } INP_UNLOCK(inp); - splx(s); return (error); } @@ -1278,4 +1243,3 @@ } return (tp); } -
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200312092313.hB9NDgwM034602>