From owner-svn-src-all@FreeBSD.ORG Sun May 8 22:15:40 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 1C9C01065670; Sun, 8 May 2011 22:15:40 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-yx0-f182.google.com (mail-yx0-f182.google.com [209.85.213.182]) by mx1.freebsd.org (Postfix) with ESMTP id 78D208FC0C; Sun, 8 May 2011 22:15:39 +0000 (UTC) Received: by yxl31 with SMTP id 31so2057624yxl.13 for ; Sun, 08 May 2011 15:15:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=BES7lP6X7HfDxF59Tz43qFljA2f/MlFepSZetdpdzks=; b=d4ExrJ+Cb6GxLGD8NWGBmkfiApms3hLMrFeV7PZ31chkwiE/YdR9JedNYtlIbUZmMo nXnIN9F7OQgr8TAR+4dM3pGr1lU8JR424QigDSji9zcMjyqdUv09Vf0w1UP6qu62G8pj 8Drb+s15i8n5uKAVA2FRcHSjNOEVVg8XVE8Mw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=fZfNOJrK2xQSo0gYKAbCXCV2RnYMEqOHXl9ZF8ICo9OJ9VI5RGmA2eXmQRxKpapw15 gph17K/8Ac78WzREsOu+JEkoAiHFPlM+9kEBbcmFv2PFnEyXDyVDGZDwYO+zcRg2R8tR uS2VJPUZnnoT14AqMbWD2pH/1V3BWAikPZjHo= MIME-Version: 1.0 Received: by 10.236.153.161 with SMTP id f21mr7119822yhk.37.1304892938528; Sun, 08 May 2011 15:15:38 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.236.111.33 with HTTP; Sun, 8 May 2011 15:15:38 -0700 (PDT) In-Reply-To: <4DC691AE.701@FreeBSD.org> References: <201105062043.p46Kh2Vs065320@svn.freebsd.org> <4DC691AE.701@FreeBSD.org> Date: Sun, 8 May 2011 18:15:38 -0400 X-Google-Sender-Auth: C6Ido5muFa1yfAZGIzMToWBQ66Y Message-ID: From: Attilio Rao To: Andreas Tobler Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, 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: Sun, 08 May 2011 22:15:40 -0000 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 =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 -> =C2=A0atomic_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 syst= em > 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_lo= ng > 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 function= s. 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 --=20 Peace can only be achieved by understanding - A. Einstein