From owner-svn-src-head@freebsd.org Mon Jul 2 14:44:33 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5977C102BBD3 for ; Mon, 2 Jul 2018 14:44:33 +0000 (UTC) (envelope-from steven@multiplay.co.uk) Received: from mail-lj1-x235.google.com (mail-lj1-x235.google.com [IPv6:2a00:1450:4864:20::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 950F58BF05 for ; Mon, 2 Jul 2018 14:44:32 +0000 (UTC) (envelope-from steven@multiplay.co.uk) Received: by mail-lj1-x235.google.com with SMTP id t21-v6so12751264lji.0 for ; Mon, 02 Jul 2018 07:44:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=multiplay-co-uk.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=crP/EXzB1XA2rkFHBrhIX2317urz0Eh3JsxrOSiXUvI=; b=TJb6C+FbFl9IkjCMYiWu65YU3vimk3O/kXh3HtCI2kQYBRI/NZDm5iHK9QEWAKTzIA r31fqS82g89wDpZyTI05F6h3h+SDh+pxor4g4Ws7g2UzBfJULj1dSy3cStZNoLtodBay v44FidbUEymP790Mz7UMW7ua8lgUmcNnTMcApOra6WJPJgSXPcbO6vQPtsyjfF8iS5En w3NzKHzu/GnG1/ahxon/Ed4JxBgaGV+5xZQpQM3y1iXDnuyaiy1mkPHLoCWUYhY+DPFS H1DZYei6leNzd6QF2zEZfyo7BXqCfy7R1sp2dqKPAKkXGwszBtvTVlt9fAT+AH+XFoIC wROg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=crP/EXzB1XA2rkFHBrhIX2317urz0Eh3JsxrOSiXUvI=; b=gzMCby/Cg/91lkUWeU2VYywss45JiYOPalm3l77EkOf/EJ2lqLAwfF/c3Govhbxoo3 aQyp1U6cZkTeB6QkFJo0dxvgtucK2UvCjR02rPW3Lq3Zag+zfSFy3Szghct0t+nhXHMH lEr02PVNnbegpnP8GoykMEoSpADK6H1WnSCptehB+Q4T7n97JXiviSGCzm8cWz/r1Y5f nx/2cSGqmHI1UX3XuE9ugcH6HesrLv3zcRHzo9T05ExAu6yLYb1L34mAXKX6LmVicZ/c NPJ2MAhNIm0be4P/zCm7iOXwdjUDlFWq0qHjXFWjAnmofu48kAl/X/N7CndGEEfNbk/m S2eQ== X-Gm-Message-State: APt69E0hYJFKDjahWx0T2vNBTcA737Qb2jAepaz9ZCMRFcO6rZwbo3Dx 4wNekQvE4heJO/KOogR+4+J7/B2QfdVN+kEyyr6LS0GJ X-Google-Smtp-Source: AAOMgpdWkhIpKV3vBIP6WbVyIqyH7wjBXYS389keVB3IdzN4XCP1b9hwXqlZcCvp+2LUwk3rSbarNeM+OXynPRKkKOI= X-Received: by 2002:a2e:869a:: with SMTP id l26-v6mr6450631lji.48.1530542670836; Mon, 02 Jul 2018 07:44:30 -0700 (PDT) MIME-Version: 1.0 References: <201807020519.w625JinG069138@repo.freebsd.org> In-Reply-To: <201807020519.w625JinG069138@repo.freebsd.org> From: Steven Hartland Date: Mon, 2 Jul 2018 15:44:19 +0100 Message-ID: Subject: Re: svn commit: r335856 - in head/sys: netinet sys To: Matt Macy Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.27 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Jul 2018 14:44:33 -0000 You have M_WAITOK and a null check in this change On Mon, 2 Jul 2018 at 06:20, Matt Macy wrote: > Author: mmacy > Date: Mon Jul 2 05:19:44 2018 > New Revision: 335856 > URL: https://svnweb.freebsd.org/changeset/base/335856 > > Log: > inpcb: don't gratuitously defer frees > > Don't defer frees in sysctl handlers. It isn't necessary > and it just confuses things. > revert: r333911, r334104, and r334125 > > Requested by: jtl > > Modified: > head/sys/netinet/ip_divert.c > head/sys/netinet/raw_ip.c > head/sys/netinet/tcp_subr.c > head/sys/netinet/udp_usrreq.c > head/sys/sys/malloc.h > > Modified: head/sys/netinet/ip_divert.c > > ============================================================================== > --- head/sys/netinet/ip_divert.c Mon Jul 2 01:30:33 2018 > (r335855) > +++ head/sys/netinet/ip_divert.c Mon Jul 2 05:19:44 2018 > (r335856) > @@ -552,7 +552,6 @@ div_detach(struct socket *so) > KASSERT(inp != NULL, ("div_detach: inp == NULL")); > INP_INFO_WLOCK(&V_divcbinfo); > INP_WLOCK(inp); > - /* XXX defer destruction to epoch_call */ > in_pcbdetach(inp); > in_pcbfree(inp); > INP_INFO_WUNLOCK(&V_divcbinfo); > @@ -632,7 +631,6 @@ static int > div_pcblist(SYSCTL_HANDLER_ARGS) > { > int error, i, n; > - struct in_pcblist *il; > struct inpcb *inp, **inp_list; > inp_gen_t gencnt; > struct xinpgen xig; > @@ -672,8 +670,9 @@ div_pcblist(SYSCTL_HANDLER_ARGS) > if (error) > return error; > > - il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb > *), M_TEMP, M_WAITOK|M_ZERO_INVARIANTS); > - inp_list = il->il_inp_list; > + inp_list = malloc(n * sizeof *inp_list, M_TEMP, M_WAITOK); > + if (inp_list == NULL) > + return ENOMEM; > > INP_INFO_RLOCK(&V_divcbinfo); > for (inp = CK_LIST_FIRST(V_divcbinfo.ipi_listhead), i = 0; inp && > i < n; > @@ -702,9 +701,14 @@ div_pcblist(SYSCTL_HANDLER_ARGS) > } else > INP_RUNLOCK(inp); > } > - il->il_count = n; > - il->il_pcbinfo = &V_divcbinfo; > - epoch_call(net_epoch_preempt, &il->il_epoch_ctx, > in_pcblist_rele_rlocked); > + INP_INFO_WLOCK(&V_divcbinfo); > + for (i = 0; i < n; i++) { > + inp = inp_list[i]; > + INP_RLOCK(inp); > + if (!in_pcbrele_rlocked(inp)) > + INP_RUNLOCK(inp); > + } > + INP_INFO_WUNLOCK(&V_divcbinfo); > > if (!error) { > /* > @@ -721,6 +725,7 @@ div_pcblist(SYSCTL_HANDLER_ARGS) > INP_INFO_RUNLOCK(&V_divcbinfo); > error = SYSCTL_OUT(req, &xig, sizeof xig); > } > + free(inp_list, M_TEMP); > return error; > } > > @@ -800,7 +805,6 @@ div_modevent(module_t mod, int type, void *unused) > break; > } > ip_divert_ptr = NULL; > - /* XXX defer to epoch_call ? */ > err = pf_proto_unregister(PF_INET, IPPROTO_DIVERT, > SOCK_RAW); > INP_INFO_WUNLOCK(&V_divcbinfo); > #ifndef VIMAGE > > Modified: head/sys/netinet/raw_ip.c > > ============================================================================== > --- head/sys/netinet/raw_ip.c Mon Jul 2 01:30:33 2018 (r335855) > +++ head/sys/netinet/raw_ip.c Mon Jul 2 05:19:44 2018 (r335856) > @@ -863,7 +863,6 @@ rip_detach(struct socket *so) > ip_rsvp_force_done(so); > if (so == V_ip_rsvpd) > ip_rsvp_done(); > - /* XXX defer to epoch_call */ > in_pcbdetach(inp); > in_pcbfree(inp); > INP_INFO_WUNLOCK(&V_ripcbinfo); > @@ -1033,7 +1032,6 @@ static int > rip_pcblist(SYSCTL_HANDLER_ARGS) > { > int error, i, n; > - struct in_pcblist *il; > struct inpcb *inp, **inp_list; > inp_gen_t gencnt; > struct xinpgen xig; > @@ -1068,8 +1066,9 @@ rip_pcblist(SYSCTL_HANDLER_ARGS) > if (error) > return (error); > > - il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb > *), M_TEMP, M_WAITOK|M_ZERO_INVARIANTS); > - inp_list = il->il_inp_list; > + inp_list = malloc(n * sizeof *inp_list, M_TEMP, M_WAITOK); > + if (inp_list == NULL) > + return (ENOMEM); > > INP_INFO_RLOCK(&V_ripcbinfo); > for (inp = CK_LIST_FIRST(V_ripcbinfo.ipi_listhead), i = 0; inp && > i < n; > @@ -1098,9 +1097,14 @@ rip_pcblist(SYSCTL_HANDLER_ARGS) > } else > INP_RUNLOCK(inp); > } > - il->il_count = n; > - il->il_pcbinfo = &V_ripcbinfo; > - epoch_call(net_epoch_preempt, &il->il_epoch_ctx, > in_pcblist_rele_rlocked); > + INP_INFO_WLOCK(&V_ripcbinfo); > + for (i = 0; i < n; i++) { > + inp = inp_list[i]; > + INP_RLOCK(inp); > + if (!in_pcbrele_rlocked(inp)) > + INP_RUNLOCK(inp); > + } > + INP_INFO_WUNLOCK(&V_ripcbinfo); > > if (!error) { > /* > @@ -1116,6 +1120,7 @@ rip_pcblist(SYSCTL_HANDLER_ARGS) > INP_INFO_RUNLOCK(&V_ripcbinfo); > error = SYSCTL_OUT(req, &xig, sizeof xig); > } > + free(inp_list, M_TEMP); > return (error); > } > > > Modified: head/sys/netinet/tcp_subr.c > > ============================================================================== > --- head/sys/netinet/tcp_subr.c Mon Jul 2 01:30:33 2018 (r335855) > +++ head/sys/netinet/tcp_subr.c Mon Jul 2 05:19:44 2018 (r335856) > @@ -943,7 +943,6 @@ deregister_tcp_functions(struct tcp_function_block *bl > rw_wunlock(&tcp_function_lock); > > VNET_LIST_RLOCK(); > - /* XXX handle */ > VNET_FOREACH(vnet_iter) { > CURVNET_SET(vnet_iter); > INP_INFO_WLOCK(&V_tcbinfo); > @@ -1733,7 +1732,6 @@ tcp_ccalgounload(struct cc_algo *unload_algo) > tmpalgo = CC_ALGO(tp); > /* NewReno does not require any > init. */ > CC_ALGO(tp) = &newreno_cc_algo; > - /* XXX defer to epoch_call */ > if (tmpalgo->cb_destroy != NULL) > > tmpalgo->cb_destroy(tp->ccv); > } > @@ -2106,7 +2104,6 @@ static int > tcp_pcblist(SYSCTL_HANDLER_ARGS) > { > int error, i, m, n, pcb_count; > - struct in_pcblist *il; > struct inpcb *inp, **inp_list; > inp_gen_t gencnt; > struct xinpgen xig; > @@ -2153,8 +2150,7 @@ tcp_pcblist(SYSCTL_HANDLER_ARGS) > if (error) > return (error); > > - il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb > *), M_TEMP, M_WAITOK|M_ZERO_INVARIANTS); > - inp_list = il->il_inp_list; > + inp_list = malloc(n * sizeof *inp_list, M_TEMP, M_WAITOK); > > INP_INFO_WLOCK(&V_tcbinfo); > for (inp = CK_LIST_FIRST(V_tcbinfo.ipi_listhead), i = 0; > @@ -2197,11 +2193,15 @@ tcp_pcblist(SYSCTL_HANDLER_ARGS) > } else > INP_RUNLOCK(inp); > } > + INP_INFO_RLOCK(&V_tcbinfo); > + for (i = 0; i < n; i++) { > + inp = inp_list[i]; > + INP_RLOCK(inp); > + if (!in_pcbrele_rlocked(inp)) > + INP_RUNLOCK(inp); > + } > + INP_INFO_RUNLOCK(&V_tcbinfo); > > - il->il_count = n; > - il->il_pcbinfo = &V_tcbinfo; > - epoch_call(net_epoch_preempt, &il->il_epoch_ctx, > in_pcblist_rele_rlocked); > - > if (!error) { > /* > * Give the user an updated idea of our state. > @@ -2217,6 +2217,7 @@ tcp_pcblist(SYSCTL_HANDLER_ARGS) > INP_LIST_RUNLOCK(&V_tcbinfo); > error = SYSCTL_OUT(req, &xig, sizeof xig); > } > + free(inp_list, M_TEMP); > return (error); > } > > > Modified: head/sys/netinet/udp_usrreq.c > > ============================================================================== > --- head/sys/netinet/udp_usrreq.c Mon Jul 2 01:30:33 2018 > (r335855) > +++ head/sys/netinet/udp_usrreq.c Mon Jul 2 05:19:44 2018 > (r335856) > @@ -837,7 +837,6 @@ udp_pcblist(SYSCTL_HANDLER_ARGS) > { > int error, i, n; > struct inpcb *inp, **inp_list; > - struct in_pcblist *il; > inp_gen_t gencnt; > struct xinpgen xig; > > @@ -875,9 +874,11 @@ udp_pcblist(SYSCTL_HANDLER_ARGS) > error = SYSCTL_OUT(req, &xig, sizeof xig); > if (error) > return (error); > - il = malloc(sizeof(struct in_pcblist) + n * sizeof(struct inpcb > *), M_TEMP, M_WAITOK|M_ZERO_INVARIANTS); > - inp_list = il->il_inp_list; > > + inp_list = malloc(n * sizeof *inp_list, M_TEMP, M_WAITOK); > + if (inp_list == NULL) > + return (ENOMEM); > + > INP_INFO_RLOCK(&V_udbinfo); > for (inp = CK_LIST_FIRST(V_udbinfo.ipi_listhead), i = 0; inp && i > < n; > inp = CK_LIST_NEXT(inp, inp_list)) { > @@ -905,9 +906,14 @@ udp_pcblist(SYSCTL_HANDLER_ARGS) > } else > INP_RUNLOCK(inp); > } > - il->il_count = n; > - il->il_pcbinfo = &V_udbinfo; > - epoch_call(net_epoch_preempt, &il->il_epoch_ctx, > in_pcblist_rele_rlocked); > + INP_INFO_WLOCK(&V_udbinfo); > + for (i = 0; i < n; i++) { > + inp = inp_list[i]; > + INP_RLOCK(inp); > + if (!in_pcbrele_rlocked(inp)) > + INP_RUNLOCK(inp); > + } > + INP_INFO_WUNLOCK(&V_udbinfo); > > if (!error) { > /* > @@ -923,6 +929,7 @@ udp_pcblist(SYSCTL_HANDLER_ARGS) > INP_INFO_RUNLOCK(&V_udbinfo); > error = SYSCTL_OUT(req, &xig, sizeof xig); > } > + free(inp_list, M_TEMP); > return (error); > } > > @@ -1714,7 +1721,6 @@ udp_detach(struct socket *so) > INP_WLOCK(inp); > up = intoudpcb(inp); > KASSERT(up != NULL, ("%s: up == NULL", __func__)); > - /* XXX defer to epoch_call */ > inp->inp_ppcb = NULL; > in_pcbdetach(inp); > in_pcbfree(inp); > > Modified: head/sys/sys/malloc.h > > ============================================================================== > --- head/sys/sys/malloc.h Mon Jul 2 01:30:33 2018 (r335855) > +++ head/sys/sys/malloc.h Mon Jul 2 05:19:44 2018 (r335856) > @@ -63,13 +63,6 @@ > > #define M_MAGIC 877983977 /* time when first defined > :-) */ > > -#ifdef INVARIANTS > -#define M_ZERO_INVARIANTS M_ZERO > -#else > -#define M_ZERO_INVARIANTS 0 > -#endif > - > - > /* > * Two malloc type structures are present: malloc_type, which is used by a > * type owner to declare the type, and malloc_type_internal, which holds > >