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

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



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

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