From nobody Tue Nov 09 18:50:29 2021 X-Original-To: dev-commits-src-main@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 8E38D1848B18; Tue, 9 Nov 2021 18:50:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HpcVs3bQCz53Wc; Tue, 9 Nov 2021 18:50:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 5B5281FB1; Tue, 9 Nov 2021 18:50:29 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1A9IoTWl062113; Tue, 9 Nov 2021 18:50:29 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1A9IoTgJ062112; Tue, 9 Nov 2021 18:50:29 GMT (envelope-from git) Date: Tue, 9 Nov 2021 18:50:29 GMT Message-Id: <202111091850.1A9IoTgJ062112@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: John Baldwin Subject: git: e3ba94d4f34e - main - Don't require the socket lock for sorele(). List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jhb X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: e3ba94d4f34e9f3a1d25396afffe2f5cbf4f5286 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by jhb: URL: https://cgit.FreeBSD.org/src/commit/?id=e3ba94d4f34e9f3a1d25396afffe2f5cbf4f5286 commit e3ba94d4f34e9f3a1d25396afffe2f5cbf4f5286 Author: John Baldwin AuthorDate: 2021-11-09 18:50:12 +0000 Commit: John Baldwin CommitDate: 2021-11-09 18:50:12 +0000 Don't require the socket lock for sorele(). Previously, sorele() always required the socket lock and dropped the lock if the released reference was not the last reference. Many callers locked the socket lock just before calling sorele() resulting in a wasted lock/unlock when not dropping the last reference. Move the previous implementation of sorele() into a new sorele_locked() function and use it instead of sorele() for various places in uipc_socket.c that called sorele() while already holding the socket lock. The sorele() macro now uses refcount_release_if_not_last() try to drop the socket reference without locking the socket. If that shortcut fails, it locks the socket and calls sorele_locked(). Reviewed by: kib, markj Sponsored by: Chelsio Communications Differential Revision: https://reviews.freebsd.org/D32741 --- sys/dev/cxgbe/tom/t4_cpl_io.c | 1 - sys/kern/kern_sendfile.c | 1 - sys/kern/sys_socket.c | 1 - sys/kern/uipc_ktls.c | 6 ------ sys/kern/uipc_socket.c | 24 +++++++++++++++++++----- sys/netinet/tcp_subr.c | 1 - sys/rpc/clnt_vc.c | 1 - sys/rpc/svc_vc.c | 1 - sys/sys/socketvar.h | 11 ++++++----- 9 files changed, 25 insertions(+), 22 deletions(-) diff --git a/sys/dev/cxgbe/tom/t4_cpl_io.c b/sys/dev/cxgbe/tom/t4_cpl_io.c index 686819065863..8635dc3d8922 100644 --- a/sys/dev/cxgbe/tom/t4_cpl_io.c +++ b/sys/dev/cxgbe/tom/t4_cpl_io.c @@ -2372,7 +2372,6 @@ t4_aiotx_task(void *context, int pending) NET_EPOCH_EXIT(et); free_toepcb(toep); - SOCK_LOCK(so); sorele(so); CURVNET_RESTORE(); } diff --git a/sys/kern/kern_sendfile.c b/sys/kern/kern_sendfile.c index ba1fc201c2de..fb5258e31f35 100644 --- a/sys/kern/kern_sendfile.c +++ b/sys/kern/kern_sendfile.c @@ -399,7 +399,6 @@ sendfile_iodone(void *arg, vm_page_t *pa, int count, int error) (void)(so->so_proto->pr_usrreqs->pru_ready)(so, sfio->m, sfio->npages); - SOCK_LOCK(so); sorele(so); #ifdef KERN_TLS out_with_ref: diff --git a/sys/kern/sys_socket.c b/sys/kern/sys_socket.c index 910da911b189..8f24017381df 100644 --- a/sys/kern/sys_socket.c +++ b/sys/kern/sys_socket.c @@ -727,7 +727,6 @@ soaio_process_sb(struct socket *so, struct sockbuf *sb) sb->sb_flags &= ~SB_AIO_RUNNING; SOCKBUF_UNLOCK(sb); - SOCK_LOCK(so); sorele(so); CURVNET_RESTORE(); } diff --git a/sys/kern/uipc_ktls.c b/sys/kern/uipc_ktls.c index f97bf9d1117f..4e14cee18c8a 100644 --- a/sys/kern/uipc_ktls.c +++ b/sys/kern/uipc_ktls.c @@ -2077,7 +2077,6 @@ deref: SOCKBUF_UNLOCK_ASSERT(sb); CURVNET_SET(so->so_vnet); - SOCK_LOCK(so); sorele(so); CURVNET_RESTORE(); } @@ -2427,7 +2426,6 @@ ktls_encrypt(struct ktls_wq *wq, struct mbuf *top) mb_free_notready(top, total_pages); } - SOCK_LOCK(so); sorele(so); CURVNET_RESTORE(); } @@ -2472,7 +2470,6 @@ ktls_encrypt_cb(struct ktls_ocf_encrypt_state *state, int error) mb_free_notready(m, npages); } - SOCK_LOCK(so); sorele(so); CURVNET_RESTORE(); } @@ -2523,7 +2520,6 @@ ktls_encrypt_async(struct ktls_wq *wq, struct mbuf *top) counter_u64_add(ktls_offload_failed_crypto, 1); free(state, M_KTLS); CURVNET_SET(so->so_vnet); - SOCK_LOCK(so); sorele(so); CURVNET_RESTORE(); break; @@ -2539,7 +2535,6 @@ ktls_encrypt_async(struct ktls_wq *wq, struct mbuf *top) mb_free_notready(m, total_pages - npages); } - SOCK_LOCK(so); sorele(so); CURVNET_RESTORE(); } @@ -2732,7 +2727,6 @@ ktls_disable_ifnet_help(void *context, int pending __unused) } out: - SOCK_LOCK(so); sorele(so); if (!in_pcbrele_wlocked(inp)) INP_WUNLOCK(inp); diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c index e033b2d77f1d..b64db8f410f3 100644 --- a/sys/kern/uipc_socket.c +++ b/sys/kern/uipc_socket.c @@ -785,7 +785,7 @@ sonewconn(struct socket *head, int connstatus) sp->so_qstate = SQ_NONE; sp->so_listen = NULL; SOCK_UNLOCK(sp); - sorele(head); /* does SOLISTEN_UNLOCK, head stays */ + sorele_locked(head); /* does SOLISTEN_UNLOCK, head stays */ soabort(sp); SOLISTEN_LOCK(head); } @@ -1090,7 +1090,7 @@ solisten_dequeue(struct socket *head, struct socket **ret, int flags) else so->so_state |= (flags & SOCK_NONBLOCK) ? SS_NBIO : 0; SOCK_UNLOCK(so); - sorele(head); + sorele_locked(head); *ret = so; return (0); @@ -1170,7 +1170,7 @@ sofree(struct socket *so) KASSERT(so->so_listen == NULL, ("%s: so %p not on (in)comp with so_listen", __func__, so)); - sorele(sol); + sorele_locked(sol); KASSERT(refcount_load(&so->so_count) == 1, ("%s: so %p count %u", __func__, so, so->so_count)); so->so_count = 0; @@ -1213,6 +1213,20 @@ sofree(struct socket *so) sodealloc(so); } +/* + * Release a reference on a socket while holding the socket lock. + * Unlocks the socket lock before returning. + */ +void +sorele_locked(struct socket *so) +{ + SOCK_LOCK_ASSERT(so); + if (refcount_release(&so->so_count)) + sofree(so); + else + SOCK_UNLOCK(so); +} + /* * Close a socket on last file table reference removal. Initiate disconnect * if connected. Free socket when disconnect complete. @@ -1282,7 +1296,7 @@ drop: } KASSERT((so->so_state & SS_NOFDREF) == 0, ("soclose: NOFDREF")); so->so_state |= SS_NOFDREF; - sorele(so); + sorele_locked(so); if (listening) { struct socket *sp, *tsp; @@ -4060,7 +4074,7 @@ soisconnected(struct socket *so) * The socket is about to soabort(). */ SOCK_UNLOCK(so); - sorele(head); + sorele_locked(head); return; } last = refcount_release(&head->so_count); diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c index 2075c1d853c3..a9f39c2c7f3d 100644 --- a/sys/netinet/tcp_subr.c +++ b/sys/netinet/tcp_subr.c @@ -3911,7 +3911,6 @@ sysctl_switch_tls(SYSCTL_HANDLER_ARGS) error = ktls_set_tx_mode(so, arg2 == 0 ? TCP_TLS_MODE_SW : TCP_TLS_MODE_IFNET); INP_WUNLOCK(inp); - SOCK_LOCK(so); sorele(so); } } else diff --git a/sys/rpc/clnt_vc.c b/sys/rpc/clnt_vc.c index 7d22c670b017..659bb4598dcb 100644 --- a/sys/rpc/clnt_vc.c +++ b/sys/rpc/clnt_vc.c @@ -912,7 +912,6 @@ clnt_vc_destroy(CLIENT *cl) } /* Must sorele() to get rid of reference. */ CURVNET_SET(so->so_vnet); - SOCK_LOCK(so); sorele(so); CURVNET_RESTORE(); } else { diff --git a/sys/rpc/svc_vc.c b/sys/rpc/svc_vc.c index 77452d906594..b94137ef1087 100644 --- a/sys/rpc/svc_vc.c +++ b/sys/rpc/svc_vc.c @@ -468,7 +468,6 @@ svc_vc_destroy_common(SVCXPRT *xprt) } /* Must sorele() to get rid of reference. */ CURVNET_SET(xprt->xp_socket->so_vnet); - SOCK_LOCK(xprt->xp_socket); sorele(xprt->xp_socket); CURVNET_RESTORE(); } else diff --git a/sys/sys/socketvar.h b/sys/sys/socketvar.h index 18db5e34d3b5..019fdfc372ac 100644 --- a/sys/sys/socketvar.h +++ b/sys/sys/socketvar.h @@ -337,11 +337,11 @@ struct socket { */ #define soref(so) refcount_acquire(&(so)->so_count) #define sorele(so) do { \ - SOCK_LOCK_ASSERT(so); \ - if (refcount_release(&(so)->so_count)) \ - sofree(so); \ - else \ - SOCK_UNLOCK(so); \ + SOCK_UNLOCK_ASSERT(so); \ + if (!refcount_release_if_not_last(&(so)->so_count)) { \ + SOCK_LOCK(so); \ + sorele_locked(so); \ + } \ } while (0) /* @@ -503,6 +503,7 @@ int soreceive_dgram(struct socket *so, struct sockaddr **paddr, int soreceive_generic(struct socket *so, struct sockaddr **paddr, struct uio *uio, struct mbuf **mp0, struct mbuf **controlp, int *flagsp); +void sorele_locked(struct socket *so); int soreserve(struct socket *so, u_long sndcc, u_long rcvcc); void sorflush(struct socket *so); int sosend(struct socket *so, struct sockaddr *addr, struct uio *uio,