Date: Wed, 18 Aug 2021 13:07:17 -0400 From: Mark Johnston <markj@freebsd.org> To: Mateusz Guzik <mjg@freebsd.org> 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: <YR0%2BRbxtskHV0uu6@nuc> In-Reply-To: <202108171959.17HJx16Z069832@gitrepo.freebsd.org> References: <202108171959.17HJx16Z069832@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <mjg@FreeBSD.org> > AuthorDate: 2021-08-15 21:41:47 +0000 > Commit: Mateusz Guzik <mjg@FreeBSD.org> > 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 <sys/kernel.h> > #include <sys/lock.h> > #include <sys/mutex.h> > +#include <sys/rmlock.h> > #include <sys/socketvar.h> > #include <sys/systm.h> > > @@ -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 <sys/queue.h> > + > /* 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"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YR0%2BRbxtskHV0uu6>