[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCHv3 1/4] x86: provide xadd()



>>> On 21.04.15 at 15:15, <david.vrabel@xxxxxxxxxx> wrote:
> On 21/04/15 14:12, Jan Beulich wrote:
>>>>> On 21.04.15 at 14:36, <david.vrabel@xxxxxxxxxx> wrote:
>>> On 21/04/15 11:36, Jan Beulich wrote:
>>>>>>> On 21.04.15 at 12:11, <david.vrabel@xxxxxxxxxx> wrote:
>>>>> +static always_inline unsigned long __xadd(
>>>>> +    volatile void *ptr, unsigned long v, int size)
>>>>> +{
>>>>> +    switch ( size )
>>>>> +    {
>>>>> +    case 1:
>>>>> +        asm volatile ( "lock; xaddb %b0,%1"
>>>>> +                       : "+r" (v), "+m" (*__xg((volatile void *)ptr))
>>>>> +                       :: "memory");
>>>>> +        return v;
>>>>
>>>> This doesn't seem to guarantee to return the old value: When the
>>>> passed in v has more than 8 significant bits (which will get ignored
>>>> as input), nothing will zap those bits from the register. Same for
>>>> the 16-bit case obviously.
>>>>
>>>>> +#define xadd(ptr, v) ({                                         \
>>>>> +            __xadd((ptr), (unsigned long)(v), sizeof(*(ptr)));  \
>>>>> +        })
>>>>
>>>> Assuming only xadd() is supposed to be used directly, perhaps
>>>> the easiest would be to cast v to typeof(*(ptr)) (instead of
>>>> unsigned long) here?
>>>
>>> I don't see how this helps.  Did you perhaps mean cast the result?
>>>
>>> #define xadd(ptr, v) ({                                    \
>>>             (typeof *(ptr))__xadd(ptr, (unsigned long)(v), \
>>>                                   sizeof(*(ptr)));         \
>>>         })
>> 
>> Casting the result would work too; casting the input would have
>> the same effect because (as said) the actual xadd doesn't alter
>> bits 8...63 (or 16...63 in the 16-bit case), i.e. whether zero
>> extension happens before or after doing the xadd doesn't matter.
> 
> Oh yes, of course.  Any preference to which method?

Not really - I guess the way you proposed it is the more obvious
one.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.