From owner-svn-src-head@freebsd.org Wed Nov 14 09:46:11 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 C6A861121CE8; Wed, 14 Nov 2018 09:46:11 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from smtp-out-no.shaw.ca (smtp-out-no.shaw.ca [64.59.134.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id D83F36F9E1; Wed, 14 Nov 2018 09:46:10 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from spqr.komquats.com ([70.67.125.17]) by shaw.ca with ESMTPA id MrkIgpHmijQc4MrkJg2W5M; Wed, 14 Nov 2018 02:46:04 -0700 X-Authority-Analysis: v=2.3 cv=bOrH382Z c=1 sm=1 tr=0 a=VFtTW3WuZNDh6VkGe7fA3g==:117 a=VFtTW3WuZNDh6VkGe7fA3g==:17 a=kj9zAlcOel0A:10 a=JHtHm7312UAA:10 a=YxBL1-UpAAAA:8 a=6I5d2MoRAAAA:8 a=VxmjJ2MpAAAA:8 a=62JuAEmFnlXWmhZl7QoA:9 a=jPecpB6AOpLXUzJM:21 a=CjuIK1q_8ugA:10 a=xb5iFRXg2qMA:10 a=Ia-lj3WSrqcvXOmTRaiG:22 a=IjZwj45LgO3ly-622nXo:22 a=7gXAzLPJhVmCkEl4_tsf:22 Received: from slippy.cwsent.com (slippy [10.1.1.91]) by spqr.komquats.com (Postfix) with ESMTPS id 4C5E5226; Wed, 14 Nov 2018 01:46:02 -0800 (PST) Received: from slippy.cwsent.com (localhost [127.0.0.1]) by slippy.cwsent.com (8.15.2/8.15.2) with ESMTP id wAE9k2rd004245; Wed, 14 Nov 2018 01:46:02 -0800 (PST) (envelope-from Cy.Schubert@cschubert.com) Received: from slippy (cy@localhost) by slippy.cwsent.com (8.15.2/8.15.2/Submit) with ESMTP id wAE9k1QM004242; Wed, 14 Nov 2018 01:46:01 -0800 (PST) (envelope-from Cy.Schubert@cschubert.com) Message-Id: <201811140946.wAE9k1QM004242@slippy.cwsent.com> X-Authentication-Warning: slippy.cwsent.com: cy owned process doing -bs X-Mailer: exmh version 2.8.0 04/21/2012 with nmh-1.7.1 Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Gleb Smirnoff , 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 In-Reply-To: Message from Cy Schubert of "Wed, 14 Nov 2018 01:33:59 -0800." <201811140933.wAE9XxSL003613@slippy.cwsent.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Wed, 14 Nov 2018 01:46:01 -0800 X-CMAE-Envelope: MS4wfIYwIlZcKmjlLzQ1X5RYClHMNKNLJ5qM18mBv/nIQ2mGGkNKCgfEy7DPXFmdceDyl+y5Muceo63n8zmI4mk9ls69fGQGEwigR7c7FZm8Fr4FMeNOZCrc UKVgtFat6dItdNcb+46MxinExi3t97X2HJyVBXoju4f6VktDq/5762bI4SESrEoULvNh2DW8eI8Xz/RGC5/9ZP84iD2NLFhQR9b6iqJlwtESEy0T4Yd7GMqF YqgvOhofbKxhqiQ/KvB/hPj6o7ZN/MBRrih+HOQ8hKhhOqqEoQ0uduo0fn/zNcIf X-Rspamd-Queue-Id: D83F36F9E1 X-Spamd-Result: default: False [-3.57 / 200.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; RCVD_COUNT_FIVE(0.00)[5]; RCVD_IN_DNSWL_LOW(-0.10)[12.134.59.64.list.dnswl.org : 127.0.5.1]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; MV_CASE(0.50)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; HAS_XAW(0.00)[]; HAS_REPLYTO(0.00)[Cy.Schubert@cschubert.com]; TO_DN_SOME(0.00)[]; REPLYTO_EQ_FROM(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; MX_GOOD(-0.01)[cached: spqr.komquats.com]; NEURAL_HAM_SHORT(-0.88)[-0.881,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; R_SPF_NA(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; RCVD_TLS_LAST(0.00)[]; ASN(0.00)[asn:6327, ipnet:64.59.128.0/20, country:CA]; IP_SCORE(-0.98)[ipnet: 64.59.128.0/20(-2.66), asn: 6327(-2.13), country: CA(-0.10)]; RECEIVED_SPAMHAUS_PBL(0.00)[17.125.67.70.zen.spamhaus.org : 127.0.0.11] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 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: Wed, 14 Nov 2018 09:46:12 -0000 Sorry. This should have been a private email. But now that it's in the wild, if_sk.c calls epoch_enter_preempt() at line 84 which causes panic early in boot. -- Cheers, Cy Schubert FreeBSD UNIX: Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few. In message <201811140933.wAE9XxSL003613@slippy.cwsent.com>, Cy Schubert writes: > 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_tracke > r. > > Embedding tracker into ifnet(9) or ifnet derived structures creates a non > > reentrable function, that will fail miserably if called simultaneously fr > om > > two different contexts. > > A thread private tracker will provide a single tracker that would allow t > o > > call these functions safely. It doesn't allow nested call, but this is no > t > > 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 > FreeBSD UNIX: Web: http://www.FreeBSD.org > > The need of the many outweighs the greed of the few. > > >