Date: Fri, 13 May 2011 17:26:44 +0200 From: Attilio Rao <attilio@freebsd.org> To: Artem Belevich <art@freebsd.org> Cc: svn-src-projects@freebsd.org, Oleksandr Tymoshenko <gonzo@freebsd.org>, src-committers@freebsd.org, Warner Losh <imp@freebsd.org>, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r221614 - projects/largeSMP/sys/powerpc/include Message-ID: <BANLkTikojmBozv5=dEni=Ec7xW19FAQMaA@mail.gmail.com> In-Reply-To: <BANLkTini3Q2MYn8e8UF2G0KbdWAnu9qWtw@mail.gmail.com> References: <201105080039.p480doiZ021493@svn.freebsd.org> <BANLkTi=e7GtBM-PTq9yJHSLRoaOWh62AeA@mail.gmail.com> <BANLkTiktwEvRktZrGOqKKB2iSB99a3Jw=g@mail.gmail.com> <BANLkTik17r-XampEdO%2BsQ7aMOL_SDyhG=g@mail.gmail.com> <BANLkTinaWDcaiZiB3G5Szoaho1jVSeniMA@mail.gmail.com> <BANLkTimj3ohmvACmvcPa3yrdsUj=4D2V3Q@mail.gmail.com> <BANLkTikSgEXZz8vjj7kuyeWQE_oKqzB8ug@mail.gmail.com> <BANLkTinHGpL5tC3-5jOPUq6bJ2Ks7j_Dww@mail.gmail.com> <BANLkTi=DOD9p-YUMm33D5ZShTjS_Q1hEvg@mail.gmail.com> <BANLkTikj%2Bszgd%2BptzD6y%2BofPs%2B8bR7Z8ew@mail.gmail.com> <BANLkTini3Q2MYn8e8UF2G0KbdWAnu9qWtw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/5/13 Artem Belevich <art@freebsd.org>:
> On Thu, May 12, 2011 at 11:12 PM, Attilio Rao <attilio@freebsd.org> 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 <sys/types.h>
>>
>> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTikojmBozv5=dEni=Ec7xW19FAQMaA>