Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 24 Jun 2011 11:18:36 -0500
From:      Nathan Whitehorn <nwhitehorn@freebsd.org>
To:        Marcel Moolenaar <marcel@xcllnt.net>
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
Message-ID:  <4E04B8DC.1020600@freebsd.org>
In-Reply-To: <1B2D07F9-3716-4581-8426-11BE78CE1D1F@xcllnt.net>
References:  <201106232221.p5NMLSFj019042@svn.freebsd.org> <1B2D07F9-3716-4581-8426-11BE78CE1D1F@xcllnt.net>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E04B8DC.1020600>