In message <202408191437.47jebhog094...@gitrepo.freebsd.org>, Mark Johnston wri tes: > The branch main has been updated by markj: > > URL: https://cgit.FreeBSD.org/src/commit/?id=fb901935f257ddcc492fe9efb605797f > 181c6597 > > commit fb901935f257ddcc492fe9efb605797f181c6597 > Author: Mark Johnston <ma...@freebsd.org> > AuthorDate: 2024-08-19 14:20:43 +0000 > Commit: Mark Johnston <ma...@freebsd.org> > CommitDate: 2024-08-19 14:37:27 +0000 > > socket: Split up sosend_generic() > > Factor out the bits that run with the sock I/O lock held into a separate > function. In this implementation, we are doing a bit more work under > the I/O lock than before. However, lock contention is only a problem > when multiple threads are transmitting on the same socket, which is an > unusual case that is not expected to perform well in any case. > > No functional change intended. > > Reviewed by: gallatin, glebius > MFC after: 2 weeks > Sponsored by: Klara, Inc. > Sponsored by: Stormshield > Differential Revision: https://reviews.freebsd.org/D46305 > --- > sys/kern/uipc_socket.c | 47 +++++++++++++++++++++++++++++------------------ > 1 file changed, 29 insertions(+), 18 deletions(-) > > diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c > index e7c4a85d5970..a5e88fc7ffc6 100644 > --- a/sys/kern/uipc_socket.c > +++ b/sys/kern/uipc_socket.c > @@ -1656,8 +1656,8 @@ out: > * counts if EINTR/ERESTART are returned. Data and control buffers are free > d > * on return. > */ > -int > -sosend_generic(struct socket *so, struct sockaddr *addr, struct uio *uio, > +static int > +sosend_generic_locked(struct socket *so, struct sockaddr *addr, struct uio * > uio, > struct mbuf *top, struct mbuf *control, int flags, struct thread *td) > { > long space; > @@ -1673,6 +1673,9 @@ sosend_generic(struct socket *so, struct sockaddr *addr > , struct uio *uio, > tls = NULL; > tls_rtype = TLS_RLTYPE_APP; > #endif > + > + SOCK_IO_SEND_ASSERT_LOCKED(so); > + > if (uio != NULL) > resid = uio->uio_resid; > else if ((top->m_flags & M_PKTHDR) != 0) > @@ -1702,10 +1705,6 @@ sosend_generic(struct socket *so, struct sockaddr *add > r, struct uio *uio, > if (control != NULL) > clen = control->m_len; > > - error = SOCK_IO_SEND_LOCK(so, SBLOCKWAIT(flags)); > - if (error) > - goto out; > - > #ifdef KERN_TLS > tls_send_flag = 0; > tls = ktls_hold(so->so_snd.sb_tls_info); > @@ -1728,7 +1727,7 @@ sosend_generic(struct socket *so, struct sockaddr *addr > , struct uio *uio, > > if (resid == 0 && !ktls_permit_empty_frames(tls)) { > error = EINVAL; > - goto release; > + goto out; > } > } > #endif > @@ -1739,13 +1738,13 @@ restart: > if (so->so_snd.sb_state & SBS_CANTSENDMORE) { > SOCKBUF_UNLOCK(&so->so_snd); > error = EPIPE; > - goto release; > + goto out; > } > if (so->so_error) { > error = so->so_error; > so->so_error = 0; > SOCKBUF_UNLOCK(&so->so_snd); > - goto release; > + goto out; > } > if ((so->so_state & SS_ISCONNECTED) == 0) { > /* > @@ -1759,7 +1758,7 @@ restart: > if (!(resid == 0 && clen != 0)) { > SOCKBUF_UNLOCK(&so->so_snd); > error = ENOTCONN; > - goto release; > + goto out; > } > } else if (addr == NULL) { > SOCKBUF_UNLOCK(&so->so_snd); > @@ -1767,7 +1766,7 @@ restart: > error = ENOTCONN; > else > error = EDESTADDRREQ; > - goto release; > + goto out; > } > } > space = sbspace(&so->so_snd); > @@ -1777,7 +1776,7 @@ restart: > clen > so->so_snd.sb_hiwat) { > SOCKBUF_UNLOCK(&so->so_snd); > error = EMSGSIZE; > - goto release; > + goto out; > } > if (space < resid + clen && > (atomic || space < so->so_snd.sb_lowat || space < clen)) { > @@ -1785,12 +1784,12 @@ restart: > (flags & (MSG_NBIO | MSG_DONTWAIT)) != 0) { > SOCKBUF_UNLOCK(&so->so_snd); > error = EWOULDBLOCK; > - goto release; > + goto out; > } > error = sbwait(so, SO_SND); > SOCKBUF_UNLOCK(&so->so_snd); > if (error) > - goto release; > + goto out; > goto restart; > } > SOCKBUF_UNLOCK(&so->so_snd); > @@ -1835,7 +1834,7 @@ restart: > ((flags & MSG_EOR) ? M_EOR : 0)); > if (top == NULL) { > error = EFAULT; /* only possible error > */ > - goto release; > + goto out; > } > space -= resid - uio->uio_resid; > resid = uio->uio_resid; > @@ -1899,12 +1898,10 @@ restart: > control = NULL; > top = NULL; > if (error) > - goto release; > + goto out; > } while (resid && space > 0); > } while (resid); > > -release: > - SOCK_IO_SEND_UNLOCK(so); > out: > #ifdef KERN_TLS > if (tls != NULL) > @@ -1917,6 +1914,20 @@ out: > return (error); > } > > +int > +sosend_generic(struct socket *so, struct sockaddr *addr, struct uio *uio, > + struct mbuf *top, struct mbuf *control, int flags, struct thread *td) > +{ > + int error; > + > + error = SOCK_IO_SEND_LOCK(so, 0); > + if (error) > + return (error); > + error = sosend_generic_locked(so, addr, uio, top, control, flags, td); > + SOCK_IO_SEND_UNLOCK(so); > + return (error); > +} > + > /* > * Send to a socket from a kernel thread. > * >
This commit broke NFS over TCP (V3 and V4). It it results in random unrecoverable hangs of an NFS share. My test is to installworld/installkernel over NFS from my main build server. All the other socket commits in this series had no adverse affect on NFS over TCP. -- Cheers, Cy Schubert <cy.schub...@cschubert.com> FreeBSD UNIX: <c...@freebsd.org> Web: https://FreeBSD.org NTP: <c...@nwtime.org> Web: https://nwtime.org e^(i*pi)+1=0
