Date: Sat, 7 May 2011 18:17:17 -0400 From: Attilio Rao <attilio@freebsd.org> To: Nathan Whitehorn <nwhitehorn@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, andreast@freebsd.org Subject: Re: svn commit: r221550 - head/sys/powerpc/conf Message-ID: <BANLkTimNC4rZvvaH5Dk%2BQpNgE90BjoQ-Sg@mail.gmail.com> In-Reply-To: <BANLkTimiKGWeJfdCQrcPvujdfuyvMX8wbQ@mail.gmail.com> References: <201105062043.p46Kh2Vs065320@svn.freebsd.org> <BANLkTimiKGWeJfdCQrcPvujdfuyvMX8wbQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/5/6 Attilio Rao <attilio@freebsd.org>: > 2011/5/6 Nathan Whitehorn <nwhitehorn@freebsd.org>: >> Author: nwhitehorn >> Date: Fri May =C2=A06 20:43:02 2011 >> New Revision: 221550 >> URL: http://svn.freebsd.org/changeset/base/221550 >> >> Log: >> =C2=A0SMP has worked perfectly for a very long time on 32-bit PowerPC on= both >> =C2=A0UP 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. Let me know, Attilio --=20 Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTimNC4rZvvaH5Dk%2BQpNgE90BjoQ-Sg>