From nobody Tue Aug 20 14:37:30 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4WpBrZ1wDsz5TFTF; Tue, 20 Aug 2024 14:37:34 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from omta001.cacentral1.a.cloudfilter.net (omta001.cacentral1.a.cloudfilter.net [3.97.99.32]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WpBrY5mL8z4pYf; Tue, 20 Aug 2024 14:37:33 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Authentication-Results: mx1.freebsd.org; none Received: from shw-obgw-4002a.ext.cloudfilter.net ([10.228.9.250]) by cmsmtp with ESMTPS id gJmBsCp1p9TOUgPzJs0nY0; Tue, 20 Aug 2024 14:37:33 +0000 Received: from spqr.komquats.com ([70.66.152.170]) by cmsmtp with ESMTPSA id gPzGsFSK02M9qgPzIsb7mO; Tue, 20 Aug 2024 14:37:32 +0000 X-Auth-User: cschuber X-Authority-Analysis: v=2.4 cv=ce5xrWDM c=1 sm=1 tr=0 ts=66c4aa2c a=y8EK/9tc/U6QY+pUhnbtgQ==:117 a=y8EK/9tc/U6QY+pUhnbtgQ==:17 a=kj9zAlcOel0A:10 a=yoJbH4e0A30A:10 a=6I5d2MoRAAAA:8 a=YxBL1-UpAAAA:8 a=EkcXrb_YAAAA:8 a=8NkmwPbCC_4RqZIh4z8A:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 a=Ia-lj3WSrqcvXOmTRaiG:22 a=LK5xJRSDVpKd5WXXoEvA:22 Received: from slippy.cwsent.com (slippy [10.1.1.91]) by spqr.komquats.com (Postfix) with ESMTP id A0905C6A; Tue, 20 Aug 2024 07:37:30 -0700 (PDT) Received: by slippy.cwsent.com (Postfix, from userid 1000) id 9BCDB1E3; Tue, 20 Aug 2024 07:37:30 -0700 (PDT) X-Mailer: exmh version 2.9.0 11/07/2018 with nmh-1.8+dev Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Mark Johnston cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: fb901935f257 - main - socket: Split up sosend_generic() In-reply-to: <202408191437.47JEbhOG094258@gitrepo.freebsd.org> References: <202408191437.47JEbhOG094258@gitrepo.freebsd.org> Comments: In-reply-to Mark Johnston message dated "Mon, 19 Aug 2024 14:37:43 +0000." List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Tue, 20 Aug 2024 07:37:30 -0700 Message-Id: <20240820143730.9BCDB1E3@slippy.cwsent.com> X-CMAE-Envelope: MS4xfCNY08Z6qVk8oaNW+L/QvsDJfxdUyndga+ZpH1pQWB5CAv+EZ8HmmAqQBlIVvy+ha4ztive9et8tdtb05+qM069PJ8jxBbqitYbmFdvEXVB7iunbfytT SsbyOGv7M5SwQlZcOLdQQ0WmOVmUAOLhywEdj017zfAbHg7XtDDFyApNYbKcCD0ywqvnJVV8HqEy5YKo802oM43Y063xNUBNVI1Jdxbmxh+o64QNzf2pNw/J BDMLgN1Zn4onbzc7hzHPb+z8ODD7QHwLs99qvCDd7mlpM3gqLWEcVJZvQANcURG7R8U+wQoRbxKzvDJkUcuCLxuwQmnRoeW5Awxv6j72huc= X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:16509, ipnet:3.96.0.0/15, country:US] X-Rspamd-Queue-Id: 4WpBrY5mL8z4pYf In message <202408191437.47JEbhOG094258@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 > AuthorDate: 2024-08-19 14:20:43 +0000 > Commit: Mark Johnston > 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 FreeBSD UNIX: Web: https://FreeBSD.org NTP: Web: https://nwtime.org e^(i*pi)+1=0