From owner-dev-commits-src-all@freebsd.org Wed Aug 18 17:07:16 2021 Return-Path: Delivered-To: dev-commits-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 C9BD2652CA9; Wed, 18 Aug 2021 17:07:16 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qt1-x82a.google.com (mail-qt1-x82a.google.com [IPv6:2607:f8b0:4864:20::82a]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GqZ844r42z4kbC; Wed, 18 Aug 2021 17:07:16 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-qt1-x82a.google.com with SMTP id x5so2072719qtq.13; Wed, 18 Aug 2021 10:07:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=GzpDQvGxr2J3YK312J/Ok9TmG5F8UfDT12PkQIT3Rws=; b=A59I2k3SblTggwx1gQxwrYfeEqeq0eTSh3wovoD4BK/ifJgvuUFFUrjq38dHZgfXIr gg708Sv/noSH+sZMSQXyPwdyTtJav16z3cnIuvrhR+33QJvCVjFqUqV1xjU1o9KoYyaC IC0uRSa551zhBZAfN27PFoPECtgNMRQgUMW8yH3+mh9IT9PdwODshyFgZRzMM90oFi2v 3V7m9LC4fRIDMk8+9lyuUc+xQkBAd2xaB7OO/QtnTRdihqAehm8vyQNvbwGJ80EZeanZ h1pMJWbvQmYBi1MbAz30xVsP6eaBjoBZ89ju7xMBdeJgyuI+pvQMU7KHHXRW4VUG57I0 7qhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=GzpDQvGxr2J3YK312J/Ok9TmG5F8UfDT12PkQIT3Rws=; b=ZS5VfzOVGwr6AuCekNpkjiFRO+ufmDwDFim59oIpF5JkWOEfQg5GkcDI+Ioc350kaJ Q3jI07kfTsmJP9Q8LpPmwC9ZBPVwJ2a9H0QlhJMDA58EQEgcBAc9xVZffh2tcLMUvjx1 KCEwD0Qp/dp6t0t8FAXunAM0/i6/VNp7fSHcVGhoBSFbpt0+3PpwlqAzGjAEuo404uRc VCvyXBbAFMoE0HNJwdpAP5xdZvEugGNnLKVXj7lpGOt8+JDE5SO3quVaO2+LRAiIAgyL U73ug4MIwOK+8EVNmFF4iHuyVDJvttowSzQVGa+c8E7vK2RBdsN4BZWzCHBu6W5cObg7 BbPw== X-Gm-Message-State: AOAM532YZxSwMrsvz32a8UNDEACeOve82CO2C7fPdoLS1fySA4v9bNtj uznWcXF970459OBO/NnIkUkxH6lfXOA= X-Google-Smtp-Source: ABdhPJyv+TXxugj7Ya+61byJi47yqkLgbhTcbCEPqme7+lzqZwC/ecmanS5ZDrewybNcRvmyszM1Hg== X-Received: by 2002:aed:2149:: with SMTP id 67mr8486935qtc.60.1629306435513; Wed, 18 Aug 2021 10:07:15 -0700 (PDT) Received: from nuc ([142.126.162.193]) by smtp.gmail.com with ESMTPSA id k8sm260669qtp.14.2021.08.18.10.07.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Aug 2021 10:07:15 -0700 (PDT) Sender: Mark Johnston Date: Wed, 18 Aug 2021 13:07:17 -0400 From: Mark Johnston To: Mateusz Guzik Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: e0a17c3f063f - main - uipc: create dedicated lists for fast and slow timeout callbacks Message-ID: References: <202108171959.17HJx16Z069832@gitrepo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202108171959.17HJx16Z069832@gitrepo.freebsd.org> X-Rspamd-Queue-Id: 4GqZ844r42z4kbC X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Aug 2021 17:07:16 -0000 On Tue, Aug 17, 2021 at 07:59:01PM +0000, Mateusz Guzik wrote: > The branch main has been updated by mjg: > > URL: https://cgit.FreeBSD.org/src/commit/?id=e0a17c3f063fd51430fb2b4f5bc667f79d2967c2 > > commit e0a17c3f063fd51430fb2b4f5bc667f79d2967c2 > Author: Mateusz Guzik > AuthorDate: 2021-08-15 21:41:47 +0000 > Commit: Mateusz Guzik > CommitDate: 2021-08-17 19:56:05 +0000 > > uipc: create dedicated lists for fast and slow timeout callbacks > > This avoids having to walk all possible protocols only to check if they > have one (vast majority does not). > > Original patch by kevans@. > > Reviewed by: kevans > Sponsored by: Rubicon Communications, LLC ("Netgate") > --- > sys/kern/uipc_domain.c | 59 +++++++++++++++++++++++++++++++++++--------------- > sys/sys/protosw.h | 4 ++++ > 2 files changed, 46 insertions(+), 17 deletions(-) > > diff --git a/sys/kern/uipc_domain.c b/sys/kern/uipc_domain.c > index b6aefec9556a..0946a2a75326 100644 > --- a/sys/kern/uipc_domain.c > +++ b/sys/kern/uipc_domain.c > @@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > #include > #include > > @@ -76,6 +77,14 @@ static struct callout pfslow_callout; > static void pffasttimo(void *); > static void pfslowtimo(void *); > > +static struct rmlock pftimo_lock; > +RM_SYSINIT(pftimo_lock, &pftimo_lock, "pftimo"); > + > +static LIST_HEAD(, protosw) pffast_list = > + LIST_HEAD_INITIALIZER(pffast_list); > +static LIST_HEAD(, protosw) pfslow_list = > + LIST_HEAD_INITIALIZER(pfslow_list); > + > struct domain *domains; /* registered protocol domains */ > int domain_init_status = 0; > static struct mtx dom_mtx; /* domain list lock */ > @@ -183,8 +192,16 @@ domain_init(void *arg) > ("Premature initialization of domain in non-default vnet")); > if (dp->dom_init) > (*dp->dom_init)(); > - for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) > + for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) { > protosw_init(pr); > + rm_wlock(&pftimo_lock); > + if (pr->pr_fasttimo != NULL) > + LIST_INSERT_HEAD(&pffast_list, pr, pr_fasttimos); > + if (pr->pr_slowtimo != NULL) > + LIST_INSERT_HEAD(&pfslow_list, pr, pr_slowtimos); > + rm_wunlock(&pftimo_lock); I think this is wrong for VNETs: each time a VNET is created, vnet_domain_init() calls domain_init() and re-inserts the callbacks into the global list. This results in a circular list, so a softclock thread just invokes callbacks in an infinite loop. > + } > + > /* > * update global information about maximums > */ > @@ -387,6 +404,13 @@ pf_proto_register(int family, struct protosw *npr) > /* Copy the new struct protosw over the spacer. */ > bcopy(npr, fpr, sizeof(*fpr)); > > + rm_wlock(&pftimo_lock); > + if (fpr->pr_fasttimo != NULL) > + LIST_INSERT_HEAD(&pffast_list, fpr, pr_fasttimos); > + if (fpr->pr_slowtimo != NULL) > + LIST_INSERT_HEAD(&pfslow_list, fpr, pr_slowtimos); > + rm_wunlock(&pftimo_lock); > + > /* Job is done, no more protection required. */ > mtx_unlock(&dom_mtx); > > @@ -447,6 +471,13 @@ pf_proto_unregister(int family, int protocol, int type) > return (EPROTONOSUPPORT); > } > > + rm_wlock(&pftimo_lock); > + if (dpr->pr_fasttimo != NULL) > + LIST_REMOVE(dpr, pr_fasttimos); > + if (dpr->pr_slowtimo != NULL) > + LIST_REMOVE(dpr, pr_slowtimos); > + rm_wunlock(&pftimo_lock); > + > /* De-orbit the protocol and make the slot available again. */ > dpr->pr_type = 0; > dpr->pr_domain = dp; > @@ -483,39 +514,33 @@ pfctlinput(int cmd, struct sockaddr *sa) > static void > pfslowtimo(void *arg) > { > + struct rm_priotracker tracker; > struct epoch_tracker et; > - struct domain *dp; > struct protosw *pr; > > + rm_rlock(&pftimo_lock, &tracker); > NET_EPOCH_ENTER(et); > - for (dp = domains; dp; dp = dp->dom_next) { > - if ((atomic_load_int(&dp->dom_flags) & DOMF_INITED) == 0) > - continue; > - atomic_thread_fence_acq(); > - for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) > - if (pr->pr_slowtimo) > - (*pr->pr_slowtimo)(); > + LIST_FOREACH(pr, &pfslow_list, pr_slowtimos) { > + (*pr->pr_slowtimo)(); > } > NET_EPOCH_EXIT(et); > + rm_runlock(&pftimo_lock, &tracker); > callout_reset(&pfslow_callout, hz/2, pfslowtimo, NULL); > } > > static void > pffasttimo(void *arg) > { > + struct rm_priotracker tracker; > struct epoch_tracker et; > - struct domain *dp; > struct protosw *pr; > > + rm_rlock(&pftimo_lock, &tracker); > NET_EPOCH_ENTER(et); > - for (dp = domains; dp; dp = dp->dom_next) { > - if ((atomic_load_int(&dp->dom_flags) & DOMF_INITED) == 0) > - continue; > - atomic_thread_fence_acq(); > - for (pr = dp->dom_protosw; pr < dp->dom_protoswNPROTOSW; pr++) > - if (pr->pr_fasttimo) > - (*pr->pr_fasttimo)(); > + LIST_FOREACH(pr, &pffast_list, pr_fasttimos) { > + (*pr->pr_fasttimo)(); > } > NET_EPOCH_EXIT(et); > + rm_runlock(&pftimo_lock, &tracker); > callout_reset(&pffast_callout, hz/5, pffasttimo, NULL); > } > diff --git a/sys/sys/protosw.h b/sys/sys/protosw.h > index 5c2fa2d4077e..a929544501f4 100644 > --- a/sys/sys/protosw.h > +++ b/sys/sys/protosw.h > @@ -35,6 +35,8 @@ > #ifndef _SYS_PROTOSW_H_ > #define _SYS_PROTOSW_H_ > > +#include > + > /* Forward declare these structures referenced from prototypes below. */ > struct kaiocb; > struct mbuf; > @@ -93,6 +95,8 @@ struct protosw { > pr_drain_t *pr_drain; /* flush any excess space possible */ > > struct pr_usrreqs *pr_usrreqs; /* user-protocol hook */ > + LIST_ENTRY(protosw) pr_fasttimos; > + LIST_ENTRY(protosw) pr_slowtimos; > }; > /*#endif*/ > > _______________________________________________ > dev-commits-src-all@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all > To unsubscribe, send any mail to "dev-commits-src-all-unsubscribe@freebsd.org"