Date: Wed, 3 Mar 2004 19:53:20 -0800 (PST) From: Robert Watson <rwatson@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 48094 for review Message-ID: <200403040353.i243rKTg078662@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=48094 Change 48094 by rwatson@rwatson_tislabs on 2004/03/03 19:53:17 Reinstate changes accidentally submerged in netperf+sockets branch, and therefore never integrated into netperf_socket: - Remove Giant from most socket-specific system calls - Use socket lock to protect some things - Use socket buffer lock to protect most other things - XXX the MAC stuff, since in its current form, there's no lock protecting the mutable so_label field, which will need to be addressed (probably by grabbing the socket lock). - Slightly different than the netperf+sockets branch changes to minimize return/whitespace/brace diffs against rwatson_netperf, which will have conditional Giant grabs where Giant was unconditionally removed in this branch. With this change, Giant should now be back off most socket system calls for IPv4. Affected files ... .. //depot/projects/netperf_socket/sys/kern/uipc_syscalls.c#4 edit Differences ... ==== //depot/projects/netperf_socket/sys/kern/uipc_syscalls.c#4 (text+ko) ==== @@ -117,10 +117,8 @@ if (error) return (error); /* An extra reference on `fp' has been held for us by falloc(). */ - mtx_lock(&Giant); error = socreate(uap->domain, &so, uap->type, uap->protocol, td->td_ucred, td); - mtx_unlock(&Giant); FILEDESC_LOCK(fdp); if (error) { if (fdp->fd_ofiles[fd] == fp) { @@ -174,10 +172,10 @@ struct socket *so; int error; - mtx_lock(&Giant); if ((error = fgetsock(td, fd, &so, NULL)) != 0) goto done2; #ifdef MAC + /* XXXRW: MAC requires socket lock? */ error = mac_check_socket_bind(td->td_ucred, so, sa); if (error) goto done1; @@ -188,7 +186,6 @@ #endif fputsock(so); done2: - mtx_unlock(&Giant); FREE(sa, M_SONAME); return (error); } @@ -208,9 +205,9 @@ struct socket *so; int error; - mtx_lock(&Giant); if ((error = fgetsock(td, uap->s, &so, NULL)) == 0) { #ifdef MAC + /* XXXRW: MAC requires socket lock? */ error = mac_check_socket_listen(td->td_ucred, so); if (error) goto done; @@ -221,7 +218,6 @@ #endif fputsock(so); } - mtx_unlock(&Giant); return(error); } @@ -260,12 +256,13 @@ goto done3; } } - mtx_lock(&Giant); error = fgetsock(td, uap->s, &head, &fflag); if (error) goto done2; s = splnet(); + SOCK_LOCK(head); if ((head->so_options & SO_ACCEPTCONN) == 0) { + SOCK_UNLOCK(head); splx(s); error = EINVAL; goto done; @@ -279,9 +276,10 @@ head->so_error = EWOULDBLOCK; break; } - error = tsleep(&head->so_timeo, PSOCK | PCATCH, + error = msleep(&head->so_timeo, SOCK_MTX(head), PSOCK | PCATCH, "accept", 0); if (error) { + SOCK_UNLOCK(head); splx(s); goto done; } @@ -289,6 +287,7 @@ if (head->so_error) { error = head->so_error; head->so_error = 0; + SOCK_UNLOCK(head); splx(s); goto done; } @@ -303,6 +302,7 @@ so = TAILQ_FIRST(&head->so_comp); TAILQ_REMOVE(&head->so_comp, so, so_list); head->so_qlen--; + SOCK_UNLOCK(head); error = falloc(td, &nfp, &fd); if (error) { @@ -312,15 +312,18 @@ * do another wakeup so some other process might * have a chance at it. */ + SOCK_LOCK(head); TAILQ_INSERT_HEAD(&head->so_comp, so, so_list); head->so_qlen++; wakeup_one(&head->so_timeo); + SOCK_UNLOCK(head); splx(s); goto done; } /* An extra reference on `nfp' has been held for us by falloc(). */ td->td_retval[0] = fd; + /* XXX lock? */ /* connection has been removed from the listen queue */ KNOTE(&head->so_rcv.sb_sel.si_note, 0); @@ -408,7 +411,6 @@ fdrop(nfp, td); fputsock(head); done2: - mtx_unlock(&Giant); done3: return (error); } @@ -473,7 +475,6 @@ int error, s; int interrupted = 0; - mtx_lock(&Giant); if ((error = fgetsock(td, fd, &so, NULL)) != 0) goto done2; if (so->so_state & SS_ISCONNECTING) { @@ -481,6 +482,7 @@ goto done1; } #ifdef MAC + /* XXXRW: MAC requires socket lock? */ error = mac_check_socket_connect(td->td_ucred, so, sa); if (error) goto bad; @@ -493,8 +495,10 @@ goto done1; } s = splnet(); + SOCK_LOCK(so); while ((so->so_state & SS_ISCONNECTING) && so->so_error == 0) { - error = tsleep(&so->so_timeo, PSOCK | PCATCH, "connec", 0); + error = msleep(&so->so_timeo, SOCK_MTX(so), PSOCK | PCATCH, + "connec", 0); if (error) { if (error == EINTR || error == ERESTART) interrupted = 1; @@ -505,6 +509,7 @@ error = so->so_error; so->so_error = 0; } + SOCK_UNLOCK(so); splx(s); bad: if (!interrupted) @@ -514,7 +519,6 @@ done1: fputsock(so); done2: - mtx_unlock(&Giant); FREE(sa, M_SONAME); return (error); } @@ -537,7 +541,6 @@ struct socket *so1, *so2; int fd, error, sv[2]; - mtx_lock(&Giant); error = socreate(uap->domain, &so1, uap->type, uap->protocol, td->td_ucred, td); if (error) @@ -609,7 +612,6 @@ free1: (void)soclose(so1); done2: - mtx_unlock(&Giant); return (error); } @@ -695,11 +697,11 @@ int iovlen; #endif - mtx_lock(&Giant); if ((error = fgetsock(td, s, &so, NULL)) != 0) goto bad2; #ifdef MAC + /* XXXRW: MAC requires socket lock? */ error = mac_check_socket_send(td->td_ucred, so); if (error) goto bad; @@ -756,7 +758,6 @@ bad: fputsock(so); bad2: - mtx_unlock(&Giant); return (error); } @@ -936,17 +937,15 @@ int iovlen; #endif - mtx_lock(&Giant); if ((error = fgetsock(td, s, &so, NULL)) != 0) { - mtx_unlock(&Giant); return (error); } #ifdef MAC + /* XXXRW: MAC requires socket lock? */ error = mac_check_socket_receive(td->td_ucred, so); if (error) { fputsock(so); - mtx_unlock(&Giant); return (error); } #endif @@ -1070,7 +1069,6 @@ } out: fputsock(so); - mtx_unlock(&Giant); if (fromsa) FREE(fromsa, M_SONAME); if (control) @@ -1285,12 +1283,10 @@ struct socket *so; int error; - mtx_lock(&Giant); if ((error = fgetsock(td, uap->s, &so, NULL)) == 0) { error = soshutdown(so, uap->how); fputsock(so); } - mtx_unlock(&Giant); return(error); } @@ -1318,7 +1314,6 @@ if (uap->valsize < 0) return (EINVAL); - mtx_lock(&Giant); if ((error = fgetsock(td, uap->s, &so, NULL)) == 0) { sopt.sopt_dir = SOPT_SET; sopt.sopt_level = uap->level; @@ -1329,7 +1324,6 @@ error = sosetopt(so, &sopt); fputsock(so); } - mtx_unlock(&Giant); return(error); } @@ -1353,7 +1347,6 @@ struct socket *so; struct sockopt sopt; - mtx_lock(&Giant); if ((error = fgetsock(td, uap->s, &so, NULL)) != 0) goto done2; if (uap->val) { @@ -1383,7 +1376,6 @@ done1: fputsock(so); done2: - mtx_unlock(&Giant); return (error); } @@ -1408,7 +1400,6 @@ socklen_t len; int error; - mtx_lock(&Giant); if ((error = fgetsock(td, uap->fdes, &so, NULL)) != 0) goto done2; error = copyin(uap->alen, &len, sizeof (len)); @@ -1442,7 +1433,6 @@ done1: fputsock(so); done2: - mtx_unlock(&Giant); return (error); } @@ -1493,7 +1483,6 @@ socklen_t len; int error; - mtx_lock(&Giant); if ((error = fgetsock(td, uap->fdes, &so, NULL)) != 0) goto done2; if ((so->so_state & (SS_ISCONNECTED|SS_ISCONFIRMING)) == 0) { @@ -1532,7 +1521,6 @@ done1: fputsock(so); done2: - mtx_unlock(&Giant); return (error); } @@ -1686,8 +1674,6 @@ int error, s, headersize = 0, headersent = 0; struct iovec *hdr_iov = NULL; - mtx_lock(&Giant); - hdtr_size = 0; /* @@ -1718,6 +1704,7 @@ } #ifdef MAC + /* XXXRW: MAC requires socket lock? */ error = mac_check_socket_send(td->td_ucred, so); if (error) goto done; @@ -1757,7 +1744,9 @@ /* * Protect against multiple writers to the socket. */ + SOCKBUF_LOCK(&so->so_snd); (void) sblock(&so->so_snd, M_WAITOK); + SOCKBUF_UNLOCK(&so->so_snd); /* * Loop through the pages in the file, starting with the requested @@ -1791,14 +1780,17 @@ * Optimize the non-blocking case by looking at the socket space * before going to the extra work of constituting the sf_buf. */ + SOCKBUF_LOCK(&so->so_snd); if ((so->so_state & SS_NBIO) && sbspace(&so->so_snd) <= 0) { if (so->so_state & SS_CANTSENDMORE) error = EPIPE; else error = EAGAIN; sbunlock(&so->so_snd); + SOCKBUF_UNLOCK(&so->so_snd); goto done; } + SOCKBUF_UNLOCK(&so->so_snd); VM_OBJECT_LOCK(obj); /* * Attempt to look up the page. @@ -1887,7 +1879,9 @@ } vm_page_unlock_queues(); VM_OBJECT_UNLOCK(obj); + SOCKBUF_LOCK(&so->so_snd); sbunlock(&so->so_snd); + SOCKBUF_UNLOCK(&so->so_snd); goto done; } vm_page_unlock_queues(); @@ -1903,7 +1897,9 @@ if (pg->wire_count == 0 && pg->object == NULL) vm_page_free(pg); vm_page_unlock_queues(); + SOCKBUF_LOCK(&so->so_snd); sbunlock(&so->so_snd); + SOCKBUF_UNLOCK(&so->so_snd); error = EINTR; goto done; } @@ -1940,6 +1936,7 @@ * Add the buffer to the socket buffer chain. */ s = splnet(); + SOCKBUF_LOCK(&so->so_snd); retry_space: /* * Make sure that the socket is still able to take more data. @@ -1961,6 +1958,7 @@ } m_freem(m); sbunlock(&so->so_snd); + SOCKBUF_UNLOCK(&so->so_snd); splx(s); goto done; } @@ -1973,6 +1971,7 @@ if (so->so_state & SS_NBIO) { m_freem(m); sbunlock(&so->so_snd); + SOCKBUF_UNLOCK(&so->so_snd); splx(s); error = EAGAIN; goto done; @@ -1992,14 +1991,20 @@ goto retry_space; } error = (*so->so_proto->pr_usrreqs->pru_send)(so, 0, m, 0, 0, td); + /* XXX: Why release and re-grab? */ + SOCKBUF_UNLOCK(&so->so_snd); splx(s); if (error) { + SOCKBUF_LOCK(&so->so_snd); sbunlock(&so->so_snd); + SOCKBUF_UNLOCK(&so->so_snd); goto done; } headersent = 1; } + SOCKBUF_LOCK(&so->so_snd); sbunlock(&so->so_snd); + SOCKBUF_UNLOCK(&so->so_snd); /* * Send trailers. Wimp out and use writev(2). @@ -2046,8 +2051,6 @@ if (m_header) m_freem(m_header); - mtx_unlock(&Giant); - if (error == ERESTART) error = EINTR;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200403040353.i243rKTg078662>