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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan


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