From owner-freebsd-stable@freebsd.org Thu Sep 22 08:25:36 2016 Return-Path: Delivered-To: freebsd-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3F0F7BE30E0 for ; Thu, 22 Sep 2016 08:25:36 +0000 (UTC) (envelope-from slw@zxy.spb.ru) Received: from zxy.spb.ru (zxy.spb.ru [195.70.199.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 01BF2E2; Thu, 22 Sep 2016 08:25:36 +0000 (UTC) (envelope-from slw@zxy.spb.ru) Received: from slw by zxy.spb.ru with local (Exim 4.86 (FreeBSD)) (envelope-from ) id 1bmzJv-000GWh-6N; Thu, 22 Sep 2016 11:25:27 +0300 Date: Thu, 22 Sep 2016 11:25:27 +0300 From: Slawa Olhovchenkov To: Konstantin Belousov Cc: John Baldwin , freebsd-stable@freebsd.org, alc@freebsd.org Subject: Re: nginx and FreeBSD11 Message-ID: <20160922082527.GX2840@zxy.spb.ru> References: <20160907191348.GD22212@zxy.spb.ru> <1823460.vTm8IvUQsF@ralph.baldwin.cx> <20160918162241.GE2960@zxy.spb.ru> <2122051.7RxZBKUSFc@ralph.baldwin.cx> <20160920065244.GO2840@zxy.spb.ru> <20160920192053.GP2840@zxy.spb.ru> <20160920201925.GI38409@kib.kiev.ua> <20160920203853.GR2840@zxy.spb.ru> <20160920211517.GJ38409@kib.kiev.ua> <20160922075933.GL38409@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160922075933.GL38409@kib.kiev.ua> User-Agent: Mutt/1.5.24 (2015-08-30) X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: slw@zxy.spb.ru X-SA-Exim-Scanned: No (on zxy.spb.ru); SAEximRunCond expanded to false X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Sep 2016 08:25:36 -0000 On Thu, Sep 22, 2016 at 10:59:33AM +0300, Konstantin Belousov wrote: > On Wed, Sep 21, 2016 at 12:15:17AM +0300, Konstantin Belousov wrote: > > > > diff --git a/sys/vm/vm_map.c b/sys/vm/vm_map.c > > > > index a23468e..f754652 100644 > > > > --- a/sys/vm/vm_map.c > > > > +++ b/sys/vm/vm_map.c > > > > @@ -481,6 +481,7 @@ vmspace_switch_aio(struct vmspace *newvm) > > > > if (oldvm == newvm) > > > > return; > > > > > > > > + spinlock_enter(); > > > > /* > > > > * Point to the new address space and refer to it. > > > > */ > > > > @@ -489,6 +490,7 @@ vmspace_switch_aio(struct vmspace *newvm) > > > > > > > > /* Activate the new mapping. */ > > > > pmap_activate(curthread); > > > > + spinlock_exit(); > > > > > > > > /* Remove the daemon's reference to the old address space. */ > > > > KASSERT(oldvm->vm_refcnt > 1, > Did you tested the patch ? I am now installed it. For success test need 2-3 days. If test failed result may be quickly. > Below is, I believe, the committable fix, of course supposing that > the patch above worked. If you want to retest it on stable/11, ignore > efirt.c chunks. and remove patch w/ spinlock? > diff --git a/sys/amd64/amd64/efirt.c b/sys/amd64/amd64/efirt.c > index f1d67f7..c883af8 100644 > --- a/sys/amd64/amd64/efirt.c > +++ b/sys/amd64/amd64/efirt.c > @@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > #include > #include > #include > @@ -301,6 +302,17 @@ efi_enter(void) > PMAP_UNLOCK(curpmap); > return (error); > } > + > + /* > + * IPI TLB shootdown handler invltlb_pcid_handler() reloads > + * %cr3 from the curpmap->pm_cr3, which would disable runtime > + * segments mappings. Block the handler's action by setting > + * curpmap to impossible value. See also comment in > + * pmap.c:pmap_activate_sw(). > + */ > + if (pmap_pcid_enabled && !invpcid_works) > + PCPU_SET(curpmap, NULL); > + > load_cr3(VM_PAGE_TO_PHYS(efi_pml4_page) | (pmap_pcid_enabled ? > curpmap->pm_pcids[PCPU_GET(cpuid)].pm_pcid : 0)); > /* > @@ -317,7 +329,9 @@ efi_leave(void) > { > pmap_t curpmap; > > - curpmap = PCPU_GET(curpmap); > + curpmap = &curproc->p_vmspace->vm_pmap; > + if (pmap_pcid_enabled && !invpcid_works) > + PCPU_SET(curpmap, curpmap); > load_cr3(curpmap->pm_cr3 | (pmap_pcid_enabled ? > curpmap->pm_pcids[PCPU_GET(cpuid)].pm_pcid : 0)); > if (!pmap_pcid_enabled) > diff --git a/sys/amd64/amd64/pmap.c b/sys/amd64/amd64/pmap.c > index 63042e4..59e1b67 100644 > --- a/sys/amd64/amd64/pmap.c > +++ b/sys/amd64/amd64/pmap.c > @@ -6842,6 +6842,7 @@ pmap_activate_sw(struct thread *td) > { > pmap_t oldpmap, pmap; > uint64_t cached, cr3; > + register_t rflags; > u_int cpuid; > > oldpmap = PCPU_GET(curpmap); > @@ -6865,16 +6866,43 @@ pmap_activate_sw(struct thread *td) > pmap == kernel_pmap, > ("non-kernel pmap thread %p pmap %p cpu %d pcid %#x", > td, pmap, cpuid, pmap->pm_pcids[cpuid].pm_pcid)); > + > + /* > + * If the INVPCID instruction is not available, > + * invltlb_pcid_handler() is used for handle > + * invalidate_all IPI, which checks for curpmap == > + * smp_tlb_pmap. Below operations sequence has a > + * window where %CR3 is loaded with the new pmap's > + * PML4 address, but curpmap value is not yet updated. > + * This causes invltlb IPI handler, called between the > + * updates, to execute as NOP, which leaves stale TLB > + * entries. > + * > + * Note that the most typical use of > + * pmap_activate_sw(), from the context switch, is > + * immune to this race, because interrupts are > + * disabled (while the thread lock is owned), and IPI > + * happends after curpmap is updated. Protect other > + * callers in a similar way, by disabling interrupts > + * around the %cr3 register reload and curpmap > + * assignment. > + */ > + if (!invpcid_works) > + rflags = intr_disable(); > + > if (!cached || (cr3 & ~CR3_PCID_MASK) != pmap->pm_cr3) { > load_cr3(pmap->pm_cr3 | pmap->pm_pcids[cpuid].pm_pcid | > cached); > if (cached) > PCPU_INC(pm_save_cnt); > } > + PCPU_SET(curpmap, pmap); > + if (!invpcid_works) > + intr_restore(rflags); > } else if (cr3 != pmap->pm_cr3) { > load_cr3(pmap->pm_cr3); > + PCPU_SET(curpmap, pmap); > } > - PCPU_SET(curpmap, pmap); > #ifdef SMP > CPU_CLR_ATOMIC(cpuid, &oldpmap->pm_active); > #else