Date: Wed, 14 Nov 2018 01:33:59 -0800 From: Cy Schubert <Cy.Schubert@cschubert.com> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r340413 - in head/sys: kern net sys Message-ID: <201811140933.wAE9XxSL003613@slippy.cwsent.com> In-Reply-To: Message from Gleb Smirnoff <glebius@FreeBSD.org> of "Tue, 13 Nov 2018 22:58:38 %2B0000." <201811132258.wADMwctL063533@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <201811132258.wADMwctL063533@repo.freebsd.org>, Gleb Smirnoff writes : > Author: glebius > Date: Tue Nov 13 22:58:38 2018 > New Revision: 340413 > URL: https://svnweb.freebsd.org/changeset/base/340413 > > Log: > For compatibility KPI functions like if_addr_rlock() that used to have > mutexes but now are converted to epoch(9) use thread-private epoch_tracker. > Embedding tracker into ifnet(9) or ifnet derived structures creates a non > reentrable function, that will fail miserably if called simultaneously from > two different contexts. > A thread private tracker will provide a single tracker that would allow to > call these functions safely. It doesn't allow nested call, but this is not > expected from compatibility KPIs. > > Reviewed by: markj > > Modified: > head/sys/kern/kern_thread.c > head/sys/kern/subr_epoch.c > head/sys/net/if.c > head/sys/net/if_var.h > head/sys/sys/epoch.h > head/sys/sys/proc.h > > Modified: head/sys/kern/kern_thread.c > ============================================================================= > = > --- head/sys/kern/kern_thread.c Tue Nov 13 22:41:20 2018 (r34041 > 2) > +++ head/sys/kern/kern_thread.c Tue Nov 13 22:58:38 2018 (r34041 > 3) > @@ -272,6 +272,7 @@ thread_init(void *mem, int size, int flags) > td->td_rlqe = NULL; > EVENTHANDLER_DIRECT_INVOKE(thread_init, td); > umtx_thread_init(td); > + epoch_thread_init(td); > td->td_kstack = 0; > td->td_sel = NULL; > return (0); > @@ -291,6 +292,7 @@ thread_fini(void *mem, int size) > turnstile_free(td->td_turnstile); > sleepq_free(td->td_sleepqueue); > umtx_thread_fini(td); > + epoch_thread_fini(td); > seltdfini(td); > } > > > Modified: head/sys/kern/subr_epoch.c > ============================================================================= > = > --- head/sys/kern/subr_epoch.c Tue Nov 13 22:41:20 2018 (r34041 > 2) > +++ head/sys/kern/subr_epoch.c Tue Nov 13 22:58:38 2018 (r34041 > 3) > @@ -667,3 +667,17 @@ in_epoch(epoch_t epoch) > { > return (in_epoch_verbose(epoch, 0)); > } > + > +void > +epoch_thread_init(struct thread *td) > +{ > + > + td->td_et = malloc(sizeof(struct epoch_tracker), M_EPOCH, M_WAITOK); > +} > + > +void > +epoch_thread_fini(struct thread *td) > +{ > + > + free(td->td_et, M_EPOCH); > +} > > Modified: head/sys/net/if.c > ============================================================================= > = > --- head/sys/net/if.c Tue Nov 13 22:41:20 2018 (r340412) > +++ head/sys/net/if.c Tue Nov 13 22:58:38 2018 (r340413) > @@ -1767,35 +1767,29 @@ if_data_copy(struct ifnet *ifp, struct if_data *ifd) > void > if_addr_rlock(struct ifnet *ifp) > { > - MPASS(*(uint64_t *)&ifp->if_addr_et == 0); > - epoch_enter_preempt(net_epoch_preempt, &ifp->if_addr_et); > + > + epoch_enter_preempt(net_epoch_preempt, curthread->td_et); > } > > void > if_addr_runlock(struct ifnet *ifp) > { > - epoch_exit_preempt(net_epoch_preempt, &ifp->if_addr_et); > -#ifdef INVARIANTS > - bzero(&ifp->if_addr_et, sizeof(struct epoch_tracker)); > -#endif > + > + epoch_exit_preempt(net_epoch_preempt, curthread->td_et); > } > > void > if_maddr_rlock(if_t ifp) > { > > - MPASS(*(uint64_t *)&ifp->if_maddr_et == 0); > - epoch_enter_preempt(net_epoch_preempt, &ifp->if_maddr_et); > + epoch_enter_preempt(net_epoch_preempt, curthread->td_et); Hi Gleb, I was wrong. It's happening here, called from line 084 in if_sk.c. ~cy > } > > void > if_maddr_runlock(if_t ifp) > { > > - epoch_exit_preempt(net_epoch_preempt, &ifp->if_maddr_et); > -#ifdef INVARIANTS > - bzero(&ifp->if_maddr_et, sizeof(struct epoch_tracker)); > -#endif > + epoch_exit_preempt(net_epoch_preempt, curthread->td_et); > } > > /* > > Modified: head/sys/net/if_var.h > ============================================================================= > = > --- head/sys/net/if_var.h Tue Nov 13 22:41:20 2018 (r340412) > +++ head/sys/net/if_var.h Tue Nov 13 22:58:38 2018 (r340413) > @@ -381,8 +381,6 @@ struct ifnet { > */ > struct netdump_methods *if_netdump_methods; > struct epoch_context if_epoch_ctx; > - struct epoch_tracker if_addr_et; > - struct epoch_tracker if_maddr_et; > > /* > * Spare fields to be added before branching a stable branch, so > > Modified: head/sys/sys/epoch.h > ============================================================================= > = > --- head/sys/sys/epoch.h Tue Nov 13 22:41:20 2018 (r340412) > +++ head/sys/sys/epoch.h Tue Nov 13 22:58:38 2018 (r340413) > @@ -82,5 +82,8 @@ void epoch_exit_preempt(epoch_t epoch, epoch_tracker_t > void epoch_enter(epoch_t epoch); > void epoch_exit(epoch_t epoch); > > +void epoch_thread_init(struct thread *); > +void epoch_thread_fini(struct thread *); > + > #endif /* _KERNEL */ > #endif /* _SYS_EPOCH_H_ */ > > Modified: head/sys/sys/proc.h > ============================================================================= > = > --- head/sys/sys/proc.h Tue Nov 13 22:41:20 2018 (r340412) > +++ head/sys/sys/proc.h Tue Nov 13 22:58:38 2018 (r340413) > @@ -193,6 +193,7 @@ struct trapframe; > struct turnstile; > struct vm_map; > struct vm_map_entry; > +struct epoch_tracker; > > /* > * XXX: Does this belong in resource.h or resourcevar.h instead? > @@ -360,6 +361,7 @@ struct thread { > int td_lastcpu; /* (t) Last cpu we were on. */ > int td_oncpu; /* (t) Which cpu we are on. */ > void *td_lkpi_task; /* LinuxKPI task struct pointer */ > + struct epoch_tracker *td_et; /* (k) compat KPI spare tracker */ > int td_pmcpend; > }; > > -- Cheers, Cy Schubert <Cy.Schubert@cschubert.com> FreeBSD UNIX: <cy@FreeBSD.org> Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811140933.wAE9XxSL003613>