From owner-svn-src-projects@FreeBSD.ORG Fri May 13 15:26:46 2011 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8B9A41065675; Fri, 13 May 2011 15:26:46 +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 DE63B8FC14; Fri, 13 May 2011 15:26:45 +0000 (UTC) Received: by yxl31 with SMTP id 31so1148145yxl.13 for ; Fri, 13 May 2011 08:26:45 -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=+4iiz7NWQjntwhG3qZnZGjDw1LtFn6oIwumq5Yiaspg=; b=mmCgPnOp4R4a40tW/8clFjI+tDWPCX+k+IOAYE8GMvwK3KSr+ADKvjmP23DcboYPr9 MvjO6wzi63MoCo0NQkKE3nQ44rkb3pI+GVojkCpJarmHaIgDlazWHvS7VFkDjYQ04id4 xiLrlSjqLFeMqWg9PcpzpGy/2USI9h7+s6KaE= 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=Ebguc9FX6y06JbpBkGzjlP03LfdlJj6a83Lq8uJwXaQDodRofvFwelyMtIAP56DebQ 58+qMpGFy/7Tmcho4xI5ALRZx3Vfh0E9P/fV21MYs/XDGnEHhUGm5IN6wILnO9lmOfC8 JZ5TTa/EP0S2ocHL5Ib5vbIh1L3nZlNhK5XGg= MIME-Version: 1.0 Received: by 10.236.161.197 with SMTP id w45mr1690628yhk.104.1305300405037; Fri, 13 May 2011 08:26:45 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.236.111.33 with HTTP; Fri, 13 May 2011 08:26:44 -0700 (PDT) In-Reply-To: References: <201105080039.p480doiZ021493@svn.freebsd.org> Date: Fri, 13 May 2011 17:26:44 +0200 X-Google-Sender-Auth: g9QK-bU3fUBq8-LKU0v8ltVh6dE Message-ID: From: Attilio Rao To: Artem Belevich Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: svn-src-projects@freebsd.org, Oleksandr Tymoshenko , src-committers@freebsd.org, Warner Losh , Bruce Evans Subject: Re: svn commit: r221614 - projects/largeSMP/sys/powerpc/include X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 May 2011 15:26:46 -0000 2011/5/13 Artem Belevich : > On Thu, May 12, 2011 at 11:12 PM, Attilio Rao wrote= : >>> Could you post definition of cpuset_t ? >>> >>> It's possible that compiler was actually correct. For instance, >>> compiler would be right to complain if cpuset_t is a packed structure, >>> even if that structure is made of a single uint32_t field. >> >> It doesn't do the atomic of =C2=A0cpuset_t, it does atomic of members of >> cpuset_t which are actually long. >> For example: >> #define CPU_OR_ATOMIC(d, s) do { =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0\ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0__size_t __i; =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 \ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0for (__i =3D 0; __i < _NCPUWORDS; __i++) =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0\ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0atomic_set_long(&= (d)->__bits[__i], =C2=A0 =C2=A0 =C2=A0\ >> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(s)= ->__bits[__i]); =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0\ >> } while (0) >> > >> It is a plain C problem, uint32_t mismatches with long also on amd64, > > Ah! Indeed, that's exactly what your problem is. uint32_t is not > necessarily long on all platforms which means that using atomic_*_long > on uint32_t variables is not going to work, even if you shut compiler > warnings up by typecasting pointers to long*. > > The way I see it, your options are: > * add explicit atomic ops for uint32_t and uint64_t > * or use long/int within cpuset_t > >> in order to demonstrate the problem check this: >> #include >> >> void >> quqa(volatile uint32_t *p) >> { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0*p =3D 10; >> } >> >> int >> main(void) >> { >> =C2=A0 =C2=A0 =C2=A0 =C2=A0long d; >> >> =C2=A0 =C2=A0 =C2=A0 =C2=A0quqa(&d); >> =C2=A0 =C2=A0 =C2=A0 =C2=A0return (0); >> } > > Well, long is not uint32_t so compiler would be correct to issue a warnin= g. > >> >>>> I compiled several kernels for MIPS (with sparse configurations and >>>> they all compile well). > > But now they will not work for N64 builds because the data you point > to would be uint32_t, but atomic_*_long() would do 64-bit loads and > stores. If you check correctly, I didn't touch the other conversion (long is not defined always the same for the 2 cases, I just modified the faulting one, the one which has the uint32_t -> long usage). I think the N64 case is fine then. > Are there any fundamental objections to extending atomic API to > include _32 and _64 ops? That's what atomic implementations do under > the hood anyways. I really don't understand your question. The atomic API already has the _32 and _64 variants. What I hoped to do is just to not rely on them in order to build standard C type versions (_int, _long, etc) in order to avoid subdle breakage as the one already reported. Attilio --=20 Peace can only be achieved by understanding - A. Einstein