From owner-svn-src-all@FreeBSD.ORG Fri Jun 24 16:18:38 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (unknown [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B71E8106566B; Fri, 24 Jun 2011 16:18:38 +0000 (UTC) (envelope-from nwhitehorn@freebsd.org) Received: from mail.icecube.wisc.edu (trout.icecube.wisc.edu [128.104.255.119]) by mx1.freebsd.org (Postfix) with ESMTP id 793EA8FC12; Fri, 24 Jun 2011 16:18:38 +0000 (UTC) Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.icecube.wisc.edu (Postfix) with ESMTP id 167995824F; Fri, 24 Jun 2011 11:18:37 -0500 (CDT) X-Virus-Scanned: amavisd-new at icecube.wisc.edu Received: from mail.icecube.wisc.edu ([127.0.0.1]) by localhost (trout.icecube.wisc.edu [127.0.0.1]) (amavisd-new, port 10030) with ESMTP id 81N7zttvvAgH; Fri, 24 Jun 2011 11:18:36 -0500 (CDT) Received: from wanderer.tachypleus.net (i3-dhcp-172-16-223-119.icecube.wisc.edu [172.16.223.119]) by mail.icecube.wisc.edu (Postfix) with ESMTP id 92BDB58141; Fri, 24 Jun 2011 11:18:36 -0500 (CDT) Message-ID: <4E04B8DC.1020600@freebsd.org> Date: Fri, 24 Jun 2011 11:18:36 -0500 From: Nathan Whitehorn User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.17) Gecko/20110516 Thunderbird/3.1.10 MIME-Version: 1.0 To: Marcel Moolenaar References: <201106232221.p5NMLSFj019042@svn.freebsd.org> <1B2D07F9-3716-4581-8426-11BE78CE1D1F@xcllnt.net> In-Reply-To: <1B2D07F9-3716-4581-8426-11BE78CE1D1F@xcllnt.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r223485 - in head/sys/powerpc: aim booke include ofw powerpc X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Jun 2011 16:18:38 -0000 On 06/24/11 11:11, Marcel Moolenaar wrote: > > On Jun 23, 2011, at 3:21 PM, Nathan Whitehorn wrote: > >> Author: nwhitehorn >> Date: Thu Jun 23 22:21:28 2011 >> New Revision: 223485 >> URL: http://svn.freebsd.org/changeset/base/223485 >> >> Log: >> Use the ABI-mandated thread pointer register (r2 for ppc32, r13 for ppc64) >> instead of a PCPU field for curthread. This averts a race on SMP systems >> with a high interrupt rate where the thread looking up the value of >> curthread could be preempted and migrated between obtaining the PCPU >> pointer and reading the value of pc_curthread, resulting in curthread being >> observed to be the current thread on the thread's original CPU. This played >> merry havoc with the system, in particular with mutexes. Many thanks to >> jhb for helping me work this one out. > > Nice catch! > > Another approach would be to have r2/r13 hold the address of the PCPU > structure and simply do a load from that address to get curthread. > The difference between the approaches is the need to to a memory load > or not for curthread. But with r2/r13 pointing to the PCPU, you may > be faster to get other PCPU fields if reading from the a SPR adds to > the overhead. Plus, it's easier to be atomic if you don't have to > read the SPR first and then do a load. The trouble with this approach is that r2/r13 would need to be updated on every CPU switch with the new PCPU pointer, so I just put curthread in there due to laziness, which is of course constant for a given thread. Another consideration is that we'd have to additionally maintain SPRG0 as the PCPU pointer anyway, since we need a PCPU area that userland can't change (r2/r13 is set from PCPU data when traps are taken now). > Is curthread the only field that needs to be atomically accessed or > are other fields in the PCPU susceptible to race conditions? In my discussion with John yesterday, he said he thought it was the only one susceptible to races of this type. The approach I used here (providing a special accessor for curthread that reads a non-PCPU-related register) is borrowed from the one used on amd64, i386, and alpha. Whether it is the only possibility for this kind of race or not, none of these platforms at least override anything else. -Nathan