From owner-svn-src-all@FreeBSD.ORG Mon May 9 13:40:56 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 282B8106564A; Mon, 9 May 2011 13:40:56 +0000 (UTC) (envelope-from andreast@FreeBSD.org) Received: from smtp.fgznet.ch (mail.fgznet.ch [81.92.96.47]) by mx1.freebsd.org (Postfix) with ESMTP id BB10A8FC17; Mon, 9 May 2011 13:40:54 +0000 (UTC) Received: from deuterium.andreas.nets (dhclient-91-190-8-131.flashcable.ch [91.190.8.131]) by smtp.fgznet.ch (8.13.8/8.13.8/Submit_SMTPAUTH) with ESMTP id p49Den1E079774; Mon, 9 May 2011 15:40:51 +0200 (CEST) (envelope-from andreast@FreeBSD.org) Message-ID: <4DC7EEE1.8000901@FreeBSD.org> Date: Mon, 09 May 2011 15:40:49 +0200 From: Andreas Tobler User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2.15) Gecko/20110303 Lightning/1.0b2 Thunderbird/3.1.9 MIME-Version: 1.0 To: Attilio Rao References: <201105062043.p46Kh2Vs065320@svn.freebsd.org> <4DC691AE.701@FreeBSD.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.64 on 81.92.96.47 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Andreas Tobler , Nathan Whitehorn Subject: Re: svn commit: r221550 - head/sys/powerpc/conf 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: Mon, 09 May 2011 13:40:56 -0000 On 09.05.11 00:15, Attilio Rao wrote: > 2011/5/8 Andreas Tobler: >> On 08.05.11 00:17, Attilio Rao wrote: >>> >>> 2011/5/6 Attilio Rao: >>>> >>>> 2011/5/6 Nathan Whitehorn: >>>>> >>>>> 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. Both builds ok and booted. Thanks, Andreas