From owner-freebsd-current@FreeBSD.ORG Wed Feb 20 21:53:13 2013 Return-Path: Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 340F5C6B; Wed, 20 Feb 2013 21:53:13 +0000 (UTC) (envelope-from ian@FreeBSD.org) Received: from duck.symmetricom.us (duck.symmetricom.us [206.168.13.214]) by mx1.freebsd.org (Postfix) with ESMTP id 0189A96C; Wed, 20 Feb 2013 21:53:12 +0000 (UTC) Received: from damnhippie.dyndns.org (daffy.symmetricom.us [206.168.13.218]) by duck.symmetricom.us (8.14.6/8.14.6) with ESMTP id r1KLrB9i028301; Wed, 20 Feb 2013 14:53:12 -0700 (MST) (envelope-from ian@FreeBSD.org) Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by damnhippie.dyndns.org (8.14.3/8.14.3) with ESMTP id r1KLqxcd051828; Wed, 20 Feb 2013 14:52:59 -0700 (MST) (envelope-from ian@FreeBSD.org) Subject: Re: [patch] i386 pmap sysmaps_pcpu[] atomic access From: Ian Lepore To: John Baldwin In-Reply-To: <201302201432.10502.jhb@freebsd.org> References: <201302201022.29294.jhb@freebsd.org> <20130220192739.GM2598@kib.kiev.ua> <201302201432.10502.jhb@freebsd.org> Content-Type: text/plain; charset="us-ascii" Date: Wed, 20 Feb 2013 14:52:59 -0700 Message-ID: <1361397179.1185.13.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: Konstantin Belousov , freebsd-current@FreeBSD.org, Svatopluk Kraus X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 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: Wed, 20 Feb 2013 21:53:13 -0000 On Wed, 2013-02-20 at 14:32 -0500, John Baldwin wrote: > On Wednesday, February 20, 2013 2:27:39 pm Konstantin Belousov wrote: > > On Wed, Feb 20, 2013 at 10:22:29AM -0500, John Baldwin wrote: > > > On Wednesday, February 20, 2013 7:31:08 am Svatopluk Kraus wrote: > > > > On Tue, Feb 19, 2013 at 7:51 PM, Konstantin Belousov > > > > wrote: > > > > > On Mon, Feb 18, 2013 at 11:18:16PM +0100, Svatopluk Kraus wrote: > > > > >> On Mon, Feb 18, 2013 at 9:36 PM, Konstantin Belousov > > > > >> wrote: > > > > >> Well, I'm taking a part on porting FreeBSD to ARM11mpcore. UP case was > > > > >> simple. SMP case is more complex and rather new for me. Recently, I > > > > >> was solving a problem with PCPU stuff. For example, PCPU_GET is > > > > >> implemented by one instruction on i386 arch. So, a need of atomicity > > > > >> with respect to interrupts can be overlooked. On load-store archs, the > > > > >> implementation which works in SMP case is not so simple. And what > > > > >> works in UP case (single PCPU), not works in SMP case. Believe me, > > > > >> mysterious and sporadic 'mutex not owned' assertions and others ones > > > > >> caused by curthreads mess, it takes a while ... > > > > > Note that PCPU_GET() is not needed to be atomic. The reason is that the code > > > > > which uses its result would not be atomic (single-instruction or whatever, see > > > > > below). Thus, either the preemption should not matter since the action with > > > > > the per-cpu data is advisory, as is in the case of i386 pmap you noted. > > > > > or some external measures should be applied in advance to the containing > > > > > region (which you proposed, by bracing with sched_pin()). > > > > > > > > So, it's advisory in the case of i386 pmap... Well, if you can live > > > > with that, I can too. > > > > > > > > > > > > > > Also, note that it is not interrupts but preemption which is concern. > > > > > > > > Yes and no. In theory, yes, a preemption is a concern. In FreeBSD, > > > > however, sched_pin() and critical_enter() and their counterparts are > > > > implemented with help of curthread. And curthread definition falls to > > > > PCPU_GET(curthread) if not defined in other way. So, curthread should > > > > be atomic with respect to interrupts and in general, PCPU_GET() should > > > > be too. Note that spinlock_enter() definitions often (always) use > > > > curthread too. Anyhow, it's defined in MD code and can be defined for > > > > each arch separately. > > > > > > curthread is a bit magic. :) If you perform a context switch during an > > > interrupt (which will change 'curthread') you also change your register state. > > > When you resume, the register state is also restored. This means that while > > > something like 'PCPU_GET(cpuid)' might be stale after you read it, 'curthread' > > > never is. However, it is true that actually reading curthread has to be > > > atomic. If your read of curthread looks like: > > > > > > mov , r0 > > > add , r0 > > > ld r0, r1 > > > > > > Then that will indeed break. Alpha used a fixed register for 'pcpu_reg' > > > (as does ia64 IIRC). OTOH, you might also be able to depend on the fact that > > > pc_curthread is the first thing in 'struct pcpu' (and always will be, you could > > > add a CTASSERT to future-proof). In that case, you can remove the 'add' > > > instruction and instead just do: > > > > > > ld , r1 > > > > > > which is fine. > > > > I just looked at the arm pcpu.h, and indeed the curthread falls > > back to the default implementation from sys/pcpu.h, which is > > get_pcpu()->pc_curthread. This looks buggy for SMP, does our arm port > > support any multi-cpu configuration ? > > Not in HEAD. There is a projects branch for armv6 I think that does. > There isn't anything I've heard of that makes it all the way to single user mode yet with multiple cores enabled. -- Ian