svn commit: r221550 - head/sys/powerpc/conf

[Attilio Rao]

2011/5/8 Andreas Tobler <andreast at freebsd.org>:
> On 08.05.11 00:17, Attilio Rao wrote:
>>
>> 2011/5/6 Attilio Rao<attilio at freebsd.org>:
>>>
>>> 2011/5/6 Nathan Whitehorn<nwhitehorn at freebsd.org>:
>>>>
>>>> Author: nwhitehorn
>>>> Date: Fri May  6 20:43:02 2011
>>>> New Revision: 221550
>>>> URL: http://svn.freebsd.org/changeset/base/221550
>>>>
>>>> Log:
>>>>  SMP has worked perfectly for a very long time on 32-bit PowerPC on both
>>>>  UP and SMP hardware. Enable it in GENERIC.
>>>>
>>>
>>> While working on largeSMP, I think there is a breakage in atomic.h.
>>> More specifically, atomic_store_rel_long() (and related functions) are
>>> not going to properly work because powerpc defines them as:
>>>
>>> atomic_store_rel_long ->  atomic_store_rel_32(volatile u_int *p, u_int v)
>>>
>>> while this should really follow the long arguments.
>>>
>>> This happens because powerpc doesn't follow the other architectures
>>> semantic on defining the "similar" atomic operations.
>>> Other arches define an hardcode version of _type version of the
>>> function and than make a macro the _32 (or whatever) version.
>>> In other words this is what they do:
>>>
>>> void
>>> atomic_store_rel_32()
>>> {
>>> ...
>>> }
>>>
>>> #define atomic_store_rel_int atomic_store_rel_32
>>>
>>> which si clearly dangerous for cases as reported above. Maybe that
>>> could be fixed by passing sized types, rather than simply int or long
>>> in numbered version, but I'd really prefer to follow the semantic by
>>> other architectures and then have:
>>>
>>> void
>>> atomic_store_rel_int()
>>> {
>>> ...
>>> }
>>>
>>> #define atomic_store_rel_32 atomic_store_rel_int
>>>
>>> I fixed the ATOMIC_STORE_LOAD case in my code, because I needed it,
>>> but the final cleanup is much bigger.
>>> I can make a patch tomorrow if you can test it.
>>>
>>
>> Can you please test and review this patch?:
>> http://www.freebsd.org/~attilio/largeSMP/atomic-powerpc.diff
>>
>> Unfortunately I'm having issues with the toolchains in atm, so I can't
>> really neither test compile it.
>
> I built kernel and world on both, on a 32-bit system and on a 64-bit system
> with 32-bit compat support.
>
> The 32-bit world failed due to type punning issues from umtx.h:121ff
>
> Attached my try to workaround these issues. With my try I can build both,
> kernel and world. Right now I have a world running with it (32-bit).
> I do not know if my try is correct. Even less after reading the comments
> from Bruce.
> But I think if it is on a somehow correct way, then we need something
> similar for the other _long functions on 32-bit where we go from the u_long
> to u_int. (e.g, atomic_add_long etc.)
>
> I'm ready for more testing.

So based on your and Bruce's feedbacks I've reworked the patch a bit:
http://www.freebsd.org/~attilio/largeSMP/atomic-powerpc2.diff

This should make type-pun correctly and avoid auto-casting on _ptr functions.

I'm sorry I couldn't even test-compile it, but I'm confident you can
fix edge cases alone.
Let me know.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein