Re: [Xen-devel] [PATCH 4/5] xen: use masking operation instead of test_bit for VPF bits

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 05/10/15 15:05, Jan Beulich wrote:
>>>> On 05.10.15 at 15:45, <george.dunlap@xxxxxxxxxx> wrote:
>> On 05/10/15 14:36, Jan Beulich wrote:
>>>>>> On 05.10.15 at 15:18, <george.dunlap@xxxxxxxxxx> wrote:
>>>> On 02/10/15 05:40, Juergen Gross wrote:
>>>>> Use a bit mask for testing of a set bit instead of test_bit in case no
>>>>> atomic operation is needed, as this will lead to smaller and more
>>>>> effective code.
>>>>>
>>>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>>>>
>>>> I'm a bit confused here -- exactly when is an atomic operation needed or
>>>> not needed?  Isn't it the case that we always need to do an atomic read
>>>> if we ever do an atomic write without a lock held?
>>>
>>> First of all - what is an atomic read from CPU perspective other than
>>> just a read? Since we talk about individual bits here, we don't care
>>> about the granularity the compiler may convert the read to, and even
>>> if the compiler chose to do multiple reads the result would still be
>>> correct, as only one of those reads can possibly have read the bit
>>> in question.
>>>
>>> And then, the old mechanism was in no way "atomic", all it added was
>>> a kind of compiler barrier (due to the cast to volatile). Yet in none of
>>> the cases changed I was able to spot a need for such a barrier.
>>
>> OK, so the key thing about test_bit isn't that it's atomic, so much that
>> it's an implicit memory barrier.  So as long as you're not doing a
>> lockless careful-ordering sort of thing, then a simple memory read
>> should be fine.  Is that correct?
> 
> Yes.
> 
>> In that case, it's likely that the patch is correct (though I'll take a
>> closer look just to be sure).
> 
> Thanks.

OK, I've looked through again and don't see anything that looks racy:

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

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