From owner-svn-src-all@freebsd.org Tue Sep 15 19:21:58 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id EC73C3DD72F; Tue, 15 Sep 2020 19:21:58 +0000 (UTC) (envelope-from markj@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 "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BrY525wY9z4WR4; Tue, 15 Sep 2020 19:21:58 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id AE7E9CEEB; Tue, 15 Sep 2020 19:21:58 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 08FJLwWa056707; Tue, 15 Sep 2020 19:21:58 GMT (envelope-from markj@FreeBSD.org) Received: (from markj@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 08FJLwCI056706; Tue, 15 Sep 2020 19:21:58 GMT (envelope-from markj@FreeBSD.org) Message-Id: <202009151921.08FJLwCI056706@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: markj set sender to markj@FreeBSD.org using -f From: Mark Johnston Date: Tue, 15 Sep 2020 19:21:58 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r365760 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: markj X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 365760 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Sep 2020 19:21:59 -0000 Author: markj Date: Tue Sep 15 19:21:58 2020 New Revision: 365760 URL: https://svnweb.freebsd.org/changeset/base/365760 Log: Improve unix socket PCB refcounting. - Use refcount_init(). - Define an INVARIANTS-only zone destructor to assert that various bits of PCB state aren't left dangling. - Annotate unp_pcb_rele() with __result_use_check. - Simplify control flow. Reviewed by: glebius, kevans, kib Tested by: pho Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D26295 Modified: head/sys/kern/uipc_usrreq.c Modified: head/sys/kern/uipc_usrreq.c ============================================================================== --- head/sys/kern/uipc_usrreq.c Tue Sep 15 19:21:33 2020 (r365759) +++ head/sys/kern/uipc_usrreq.c Tue Sep 15 19:21:58 2020 (r365760) @@ -313,25 +313,25 @@ static void unp_process_defers(void * __unused, int); static void unp_pcb_hold(struct unpcb *unp) { - MPASS(unp->unp_refcount); - refcount_acquire(&unp->unp_refcount); + u_int old __unused; + + old = refcount_acquire(&unp->unp_refcount); + KASSERT(old > 0, ("%s: unpcb %p has no references", __func__, unp)); } -static int +static __result_use_check bool unp_pcb_rele(struct unpcb *unp) { - int freed; + bool ret; UNP_PCB_LOCK_ASSERT(unp); - MPASS(unp->unp_refcount); - if ((freed = refcount_release(&unp->unp_refcount))) { - /* we got here with having detached? */ - MPASS(unp->unp_socket == NULL); + + if ((ret = refcount_release(&unp->unp_refcount))) { UNP_PCB_UNLOCK(unp); UNP_PCB_LOCK_DESTROY(unp); uma_zfree(unp_zone, unp); } - return (freed); + return (ret); } static void @@ -514,7 +514,7 @@ uipc_attach(struct socket *so, int proto, struct threa UNP_PCB_LOCK_INIT(unp); unp->unp_socket = so; so->so_pcb = unp; - unp->unp_refcount = 1; + refcount_init(&unp->unp_refcount, 1); if ((locked = UNP_LINK_WOWNED()) == false) UNP_LINK_WLOCK(); @@ -1814,7 +1814,7 @@ unp_pcblist(SYSCTL_HANDLER_ARGS) struct unp_head *head; struct xunpcb *xu; u_int i; - int error, freeunp, n; + int error, n; switch ((intptr_t)arg1) { case SOCK_STREAM: @@ -1891,9 +1891,10 @@ unp_pcblist(SYSCTL_HANDLER_ARGS) for (i = 0; i < n; i++) { unp = unp_list[i]; UNP_PCB_LOCK(unp); - freeunp = unp_pcb_rele(unp); + if (unp_pcb_rele(unp)) + continue; - if (freeunp == 0 && unp->unp_gencnt <= gencnt) { + if (unp->unp_gencnt <= gencnt) { xu->xu_len = sizeof *xu; xu->xu_unpp = (uintptr_t)unp; /* @@ -1920,8 +1921,9 @@ unp_pcblist(SYSCTL_HANDLER_ARGS) sotoxsocket(unp->unp_socket, &xu->xu_socket); UNP_PCB_UNLOCK(unp); error = SYSCTL_OUT(req, xu, sizeof *xu); - } else if (freeunp == 0) + } else { UNP_PCB_UNLOCK(unp); + } } free(xu, M_TEMP); if (!error) { @@ -2137,18 +2139,44 @@ unp_zone_change(void *tag) uma_zone_set_max(unp_zone, maxsockets); } +#ifdef INVARIANTS static void +unp_zdtor(void *mem, int size __unused, void *arg __unused) +{ + struct unpcb *unp; + + unp = mem; + + KASSERT(LIST_EMPTY(&unp->unp_refs), + ("%s: unpcb %p has lingering refs", __func__, unp)); + KASSERT(unp->unp_socket == NULL, + ("%s: unpcb %p has socket backpointer", __func__, unp)); + KASSERT(unp->unp_vnode == NULL, + ("%s: unpcb %p has vnode references", __func__, unp)); + KASSERT(unp->unp_conn == NULL, + ("%s: unpcb %p is still connected", __func__, unp)); + KASSERT(unp->unp_addr == NULL, + ("%s: unpcb %p has leaked addr", __func__, unp)); +} +#endif + +static void unp_init(void) { + uma_dtor dtor; #ifdef VIMAGE if (!IS_DEFAULT_VNET(curvnet)) return; #endif - unp_zone = uma_zcreate("unpcb", sizeof(struct unpcb), NULL, NULL, + +#ifdef INVARIANTS + dtor = unp_zdtor; +#else + dtor = NULL; +#endif + unp_zone = uma_zcreate("unpcb", sizeof(struct unpcb), NULL, dtor, NULL, NULL, UMA_ALIGN_CACHE, 0); - if (unp_zone == NULL) - panic("unp_init"); uma_zone_set_max(unp_zone, maxsockets); uma_zone_set_warning(unp_zone, "kern.ipc.maxsockets limit reached"); EVENTHANDLER_REGISTER(maxsockets_change, unp_zone_change,