2011/5/8 Andreas Tobler <andre...@freebsd.org>: > On 08.05.11 00:17, Attilio Rao wrote: >> >> 2011/5/6 Attilio Rao<atti...@freebsd.org>: >>> >>> 2011/5/6 Nathan Whitehorn<nwhiteh...@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 _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
