Re: [Xen-devel] [PATCH for-4.5 v6 17/17] tools/tests: Enable xen-access on ARM

["Aravindh Puthiyaparambil (aravindp)" <aravindp@xxxxxxxxx>]

>> On Thu, Sep 18, 2014 at 9:02 PM, Ian Campbell
>> <ian.campbell@xxxxxxxxxx> wrote:
>>         On Mon, 2014-09-15 at 16:02 +0200, Tamas K Lengyel wrote:
>>         > +static inline int test_and_set_bit(int nr, volatile void
>>         *addr)
>>         > +{
>>         > +        unsigned int mask = BIT_MASK(nr);
>>         > +        volatile unsigned int *p =
>>         > +                ((volatile unsigned int *)addr) +
>>         BIT_WORD(nr);
>>         > +        unsigned int old = *p;
>>         > +
>>         > +        *p = old | mask;
>>         > +        return (old & mask) != 0;
>>         > +}
>>
>>         This doesn't need to be / is not intended to be atomic, right?
>>
>>
>> This function I took from xen/include/asm-arm/bitops.h directly. Since
>> the x86 assembly is missing "lock;" I assume this doesn't need to be
>> atomic but I may be wrong.
>
>Looks like you have taken __test_and_set_bit (the non-atomic version)
>and renamed it. Plain test_and_set_bit is supposed to be atomic I think.
>
>>  However, in case the x86 assembly should actually have "lock;"
>> similarly to how it is in xen/include/asm-x86/bitops.h, then the ARM
>> side should be atomic too AFAIU.
>
>I don't know how xen-access.c uses them so I don't know if they are
>supposed to be atomic or not, but the comment above the x86 version says
>they are and then defines one which is not... No idea if it is the
>comment or the code which is wrong though.
>
>It seems this is used to define some sort of lock spinlock
>implementation. The code isn't multithreaded so I don't know why a lock
>is needed (or if it is why a normal userspace pthread lock isn't good
>enough). I hope this isn't sharing a lock with the hypervisor or
>something! Maybe all that locking can just be ditched, or converted to a
>standard lock.
>
>CCing Aravindh who's been touching this most recently.

Sorry about the delay in replying. I was not involved when this code was 
introduced by Joe Epstein who was the original author and I think was reviewed 
Tim. 

Tim, do you remember why this was done?

Thanks,
Aravindh

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