From owner-freebsd-current@FreeBSD.ORG Thu Feb 21 15:50:21 2013 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 7D57B29A for ; Thu, 21 Feb 2013 15:50:21 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 2B826F51 for ; Thu, 21 Feb 2013 15:50:21 +0000 (UTC) Received: from pakbsde14.localnet (unknown [38.105.238.108]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 80857B990; Thu, 21 Feb 2013 10:50:20 -0500 (EST) From: John Baldwin To: Svatopluk Kraus Subject: Re: [patch] i386 pmap sysmaps_pcpu[] atomic access Date: Thu, 21 Feb 2013 10:43:49 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p25; KDE/4.5.5; amd64; ; ) References: <201302201022.29294.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201302211043.49906.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Thu, 21 Feb 2013 10:50:20 -0500 (EST) Cc: Konstantin Belousov , freebsd-current@freebsd.org 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: Thu, 21 Feb 2013 15:50:21 -0000 On Thursday, February 21, 2013 7:53:52 am Svatopluk Kraus wrote: > On Wed, Feb 20, 2013 at 4:22 PM, 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. > > Just for the record. There are three extra (coprocessor) registers per > a core in arm11mpcore (armv6k). Unfortunately only one is Privileged. > The other ones are respectively User Read only and User Read Write. > For now, we are using the Privileged one to store pcpu pointer > (curthread is correctly set during each context switch). Thus, using a > coprocessor register means that reading of curthread is not a single > instruction implementation now. After we figured out the curthread > issue in SMP case, using a fixed (processor) register for pcpu is an > option. Meanwhile, we disable interrupts before reading of curthread > and enable them after. The same is done for other PCPU stuff. For now > we have not stable system enough for profiling, however, when it will > be, it would be interesting to learn how various implementations of > curthread reading impact system performance. curthread is read _a lot_, so I would expect this to hurt. What we did on Alpha was to use a fixed register for pcpu access, but we used the equivalent of a coprocessor register to also store the value so we could set that fixed register on entry to the kernel (userland was free to use the register for its own purposes). -- John Baldwin