From owner-freebsd-current@FreeBSD.ORG Tue May 17 16:20:46 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9518C1065673; Tue, 17 May 2011 16:20:46 +0000 (UTC) (envelope-from max@love2party.net) Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.17.9]) by mx1.freebsd.org (Postfix) with ESMTP id 272B98FC16; Tue, 17 May 2011 16:20:45 +0000 (UTC) Received: from [10.54.190.172] (gw-105.extranet.sea01.isilon.com [74.85.160.105]) by mrelayeu.kundenserver.de (node=mrbap1) with ESMTP (Nemesis) id 0LrqPu-1Ph2Em3AzO-0130km; Tue, 17 May 2011 18:20:45 +0200 Message-ID: <4DD2A058.6050400@love2party.net> Date: Tue, 17 May 2011 09:20:40 -0700 From: Max Laier User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.15) Gecko/20110419 Lightning/1.0b2pre Thunderbird/3.1.9 MIME-Version: 1.0 To: John Baldwin References: <4DCD357D.6000109@FreeBSD.org> <201105161738.53366.max@love2party.net> <201105161805.51188.max@love2party.net> <4DD26720.3000001@FreeBSD.org> In-Reply-To: <4DD26720.3000001@FreeBSD.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Provags-ID: V02:K0:pSf713jrCwEO/x85+AO8ETnUL95ISZAe1a6Exk40nOF s/m9q5zNeEkttabSG8t0KT1kCCoMo8mqfkwR38oE2F7+3ognQl n8hkceb0SFYT06Z+2jB5y5rH0wfBo//hYgv0w5WJZEzsEo0qFu iURY4VpyYe07DjQ7wIZJGoAMzMiWlJnRjzaiaQ9APLFETjWxnm vHlvhlQvZYovs1u9nonjA== Cc: neel@freebsd.org, Andriy Gapon , Attilio Rao , FreeBSD current , Stephan Uphoff , Peter Grehan Subject: Re: proposed smp_rendezvous change X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 May 2011 16:20:46 -0000 On 05/17/2011 05:16 AM, John Baldwin wrote: ... > Index: kern/kern_switch.c > =================================================================== > --- kern/kern_switch.c (revision 221536) > +++ kern/kern_switch.c (working copy) > @@ -192,15 +192,22 @@ > critical_exit(void) > { > struct thread *td; > - int flags; > + int flags, owepreempt; > > td = curthread; > KASSERT(td->td_critnest != 0, > ("critical_exit: td_critnest == 0")); > > if (td->td_critnest == 1) { > + owepreempt = td->td_owepreempt; > + td->td_owepreempt = 0; > + /* > + * XXX: Should move compiler_memory_barrier() from > + * rmlock to a header. > + */ XXX: If we get an interrupt at this point and td_owepreempt was zero, the new interrupt will re-set it, because td_critnest is still non-zero. So we still end up with a thread that is leaking an owepreempt *and* lose a preemption. We really need an atomic_readandclear() which gives us a local copy of td_owepreempt *and* clears critnest in the same operation. Sadly, that is rather expensive. It is possible to implement with a flag for owepreempt, but that means that all writes to critnest must then be atomic. Either because we know we have interrupts disabled (i.e. setting owepreempt can be a RMW), or with a proper atomic_add/set/... I'm not sure what the performance impact of this will be. One would hope that atomic_add without a memory barrier isn't much more expensive than a compiler generated read-modify-write, tho. Especially, since this cacheline should be local and exclusive to us, anyway. > + __asm __volatile("":::"memory"); > td->td_critnest = 0; > - if (td->td_owepreempt) { > + if (owepreempt) { > td->td_critnest = 1; > thread_lock(td); > td->td_critnest--;