svn commit: r221614 - projects/largeSMP/sys/powerpc/include

[Attilio Rao]

2011/5/13 Artem Belevich <art at freebsd.org>:
> On Thu, May 12, 2011 at 11:12 PM, Attilio Rao <attilio at 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  cpuset_t, it does atomic of members of
>> cpuset_t which are actually long.
>> For example:
>> #define CPU_OR_ATOMIC(d, s) do {                        \
>>        __size_t __i;                                   \
>>        for (__i = 0; __i < _NCPUWORDS; __i++)          \
>>                atomic_set_long(&(d)->__bits[__i],      \
>>                    (s)->__bits[__i]);                  \
>> } 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)
>> {
>>        *p = 10;
>> }
>>
>> int
>> main(void)
>> {
>>        long d;
>>
>>        quqa(&d);
>>        return (0);
>> }
>
> Well, long is not uint32_t so compiler would be correct to issue a warning.
>
>>
>>>> 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


-- 
Peace can only be achieved by understanding - A. Einstein