On 18/05/16 23:52, Emilio G. Cota wrote:
> On Wed, May 18, 2016 at 22:51:09 +0300, Sergey Fedorov wrote:
>> On 14/05/16 06:34, Emilio G. Cota wrote:
>>> +static inline void qemu_spin_lock(QemuSpin *spin)
>>> +{
>>> +while (atomic_test_and_set_acquire(&spin->value)) {
>> A possible optimization
On Wed, May 18, 2016 at 22:51:09 +0300, Sergey Fedorov wrote:
> On 14/05/16 06:34, Emilio G. Cota wrote:
> > +static inline void qemu_spin_lock(QemuSpin *spin)
> > +{
> > +while (atomic_test_and_set_acquire(&spin->value)) {
>
> A possible optimization might be using unlikely() here, copmare:
On 14/05/16 06:34, Emilio G. Cota wrote:
> +static inline void qemu_spin_lock(QemuSpin *spin)
> +{
> +while (atomic_test_and_set_acquire(&spin->value)) {
A possible optimization might be using unlikely() here, copmare:
spin.o: file format elf64-littleaarch64
Disassembly of section .text
On Wed, May 18, 2016 at 21:21:26 +0300, Sergey Fedorov wrote:
> On 14/05/16 06:34, Emilio G. Cota wrote:
> > +static inline int qemu_spin_trylock(QemuSpin *spin)
> > +{
> > +if (atomic_test_and_set_acquire(&spin->value)) {
> > +return -EBUSY;
>
> Seems this should be:
>
> return E
On 14/05/16 06:34, Emilio G. Cota wrote:
> +static inline int qemu_spin_trylock(QemuSpin *spin)
> +{
> +if (atomic_test_and_set_acquire(&spin->value)) {
> +return -EBUSY;
Seems this should be:
return EBUSY;
> +}
> +return 0;
> +}
On 18/05/2016 18:59, Emilio G. Cota wrote:
> On Wed, May 18, 2016 at 17:09:48 +0200, Paolo Bonzini wrote:
>> But honestly I think it would be even better to just use
>> __sync_lock_test_and_set in the spinlock implementation and not add this
>> to atomics.h. There's already enough issues with th
On Wed, May 18, 2016 at 17:09:48 +0200, Paolo Bonzini wrote:
> But honestly I think it would be even better to just use
> __sync_lock_test_and_set in the spinlock implementation and not add this
> to atomics.h. There's already enough issues with the current subset of
> atomics, I am not really hap
On 05/18/2016 08:35 AM, Peter Maydell wrote:
(v7 probably gets compiled the same way as v6 here, though.)
The meaningful difference in v7 is ldrexb, which goes to the byte semantics of
__atomic_test_and_set, which is how this digression got started.
r~
On 18/05/16 18:44, Peter Maydell wrote:
> On 18 May 2016 at 16:36, Paolo Bonzini wrote:
>> On 18/05/2016 17:35, Peter Maydell wrote:
> $ arm-linux-gnueabi-gcc -march=armv6 -O2 -c a.c
>>> I don't think armv6 is a sufficiently common host for us to
>>> worry too much about how its atomic primiti
On 18 May 2016 at 16:36, Paolo Bonzini wrote:
> On 18/05/2016 17:35, Peter Maydell wrote:
>>> > $ arm-linux-gnueabi-gcc -march=armv6 -O2 -c a.c
>> I don't think armv6 is a sufficiently common host for us to
>> worry too much about how its atomic primitives come out.
>> ARMv7 and 64-bit ARMv8 are m
On 18/05/2016 17:35, Peter Maydell wrote:
>> > $ arm-linux-gnueabi-gcc -march=armv6 -O2 -c a.c
> I don't think armv6 is a sufficiently common host for us to
> worry too much about how its atomic primitives come out.
> ARMv7 and 64-bit ARMv8 are more relevant, I think.
> (v7 probably gets compiled
On 18 May 2016 at 16:05, Sergey Fedorov wrote:
> $ arm-linux-gnueabi-gcc -march=armv6 -O2 -c a.c
I don't think armv6 is a sufficiently common host for us to
worry too much about how its atomic primitives come out.
ARMv7 and 64-bit ARMv8 are more relevant, I think.
(v7 probably gets compiled the s
On 18/05/2016 17:05, Sergey Fedorov wrote:
> Please look at this:
>
> $ cat >a.c < int atomic_exchange(int *x, int v)
> {
> return __atomic_exchange_n(x, v, __ATOMIC_ACQUIRE);
> }
>
> int sync_lock_test_and_set(int *x, int v)
> {
> __sync_lock_test_and_set(x, v);
> }
> EOF
>
> Disassem
On 18/05/16 17:59, Paolo Bonzini wrote:
>
> On 18/05/2016 16:47, Sergey Fedorov wrote:
>> Why not? AFAIK the reason to avoid __sync primitives is that in most
>> cases
>> they include barriers that callers might not necessarily need; __atomic's
>> allow for finer tuning, which is i
On 18/05/2016 16:47, Sergey Fedorov wrote:
>>> >> Why not? AFAIK the reason to avoid __sync primitives is that in most
>>> >> cases
>>> >> they include barriers that callers might not necessarily need; __atomic's
>>> >> allow for finer tuning, which is in general a good thing. However,
>>> >> __
On 18/05/16 17:18, Sergey Fedorov wrote:
> On 18/05/16 03:28, Emilio G. Cota wrote:
>> On Tue, May 17, 2016 at 23:20:11 +0300, Sergey Fedorov wrote:
>>> On 17/05/16 23:04, Emilio G. Cota wrote:
>> (snip)
+/*
+ * We might we tempted to use __atomic_test_and_set with __ATOMIC_ACQUIRE;
On 18/05/2016 16:10, Sergey Fedorov wrote:
> On 18/05/16 17:05, Paolo Bonzini wrote:
>> this:
>>
>> if (atomic_read(&x) != 1) {
>> atomic_set(&x, 1);
>> }
>>
>> couldn't become an unconditional
>>
>> atomic_set(&x, 1);
>
> Sorry, I can't figure out why it couldn't...
Because atomics c
On 18/05/16 03:28, Emilio G. Cota wrote:
> On Tue, May 17, 2016 at 23:20:11 +0300, Sergey Fedorov wrote:
>> On 17/05/16 23:04, Emilio G. Cota wrote:
> (snip)
>>> +/*
>>> + * We might we tempted to use __atomic_test_and_set with __ATOMIC_ACQUIRE;
>>> + * however, the documentation explicitly says th
On 18/05/16 17:05, Paolo Bonzini wrote:
>
> On 18/05/2016 15:59, Sergey Fedorov wrote:
>> But actually (cf include/qemu/atomic.h) we can have:
>>
>> #define atomic_read(ptr) \
>> ({\
>> QEMU_BUILD_BUG_O
On 18/05/2016 15:59, Sergey Fedorov wrote:
>
> But actually (cf include/qemu/atomic.h) we can have:
>
> #define atomic_read(ptr) \
> ({\
> QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
>
On 18/05/16 02:18, Emilio G. Cota wrote:
> On Tue, May 17, 2016 at 23:35:57 +0300, Sergey Fedorov wrote:
>> On 17/05/16 22:38, Emilio G. Cota wrote:
>>> On Tue, May 17, 2016 at 20:13:24 +0300, Sergey Fedorov wrote:
On 14/05/16 06:34, Emilio G. Cota wrote:
>> (snip)
> +while (atomic
On 05/17/2016 10:13 AM, Sergey Fedorov wrote:
>> > +static inline void qemu_spin_lock(QemuSpin *spin)
>> > +{
>> > +while (atomic_test_and_set_acquire(&spin->value)) {
>>From gcc-4.8 info page, node "__atomic Builtins", description of
> __atomic_test_and_set():
>
> It should be only used f
On Tue, May 17, 2016 at 12:19:27 -0700, Richard Henderson wrote:
> On 05/17/2016 10:13 AM, Sergey Fedorov wrote:
> >> > +static inline void qemu_spin_lock(QemuSpin *spin)
> >> > +{
> >> > +while (atomic_test_and_set_acquire(&spin->value)) {
> >>From gcc-4.8 info page, node "__atomic Builtins",
On 17/05/16 23:04, Emilio G. Cota wrote:
> On Tue, May 17, 2016 at 12:19:27 -0700, Richard Henderson wrote:
>> On 05/17/2016 10:13 AM, Sergey Fedorov wrote:
> +static inline void qemu_spin_lock(QemuSpin *spin)
> +{
> +while (atomic_test_and_set_acquire(&spin->value)) {
>>> >From gcc
On Tue, May 17, 2016 at 20:13:24 +0300, Sergey Fedorov wrote:
> On 14/05/16 06:34, Emilio G. Cota wrote:
(snip)
> > +static inline void qemu_spin_lock(QemuSpin *spin)
> > +{
> > +while (atomic_test_and_set_acquire(&spin->value)) {
>
> From gcc-4.8 info page, node "__atomic Builtins", descripti
On 17/05/16 22:19, Richard Henderson wrote:
> On 05/17/2016 10:13 AM, Sergey Fedorov wrote:
+static inline void qemu_spin_lock(QemuSpin *spin)
+{
+while (atomic_test_and_set_acquire(&spin->value)) {
>> >From gcc-4.8 info page, node "__atomic Builtins", description of
>> __atomic_
On 17/05/16 22:57, Sergey Fedorov wrote:
> On 17/05/16 22:19, Richard Henderson wrote:
>> On 05/17/2016 10:13 AM, Sergey Fedorov wrote:
> +static inline void qemu_spin_lock(QemuSpin *spin)
> +{
> +while (atomic_test_and_set_acquire(&spin->value)) {
>>> >From gcc-4.8 info page, node
On 17/05/16 22:38, Emilio G. Cota wrote:
> On Tue, May 17, 2016 at 20:13:24 +0300, Sergey Fedorov wrote:
>> On 14/05/16 06:34, Emilio G. Cota wrote:
(snip)
>>> +while (atomic_read(&spin->value)) {
>>> +cpu_relax();
>>> +}
>>> +}
>> Looks like relaxed atomic access ca
On Tue, May 17, 2016 at 23:35:57 +0300, Sergey Fedorov wrote:
> On 17/05/16 22:38, Emilio G. Cota wrote:
> > On Tue, May 17, 2016 at 20:13:24 +0300, Sergey Fedorov wrote:
> >> On 14/05/16 06:34, Emilio G. Cota wrote:
> (snip)
> >>> +while (atomic_read(&spin->value)) {
> >>> +cpu
On 05/17/2016 01:01 PM, Sergey Fedorov wrote:
>> Sorry, I can't see reading ARMv6 ARM that 1-byte access can't be atomic. What
>> I've found:
>>
>> B2.4.1 Normal memory attribute
>> (snip)
>> Shared Normal memory
>>
>> (snip)
>> ... Reads to Shared Normal Memory that are
On 05/17/2016 03:12 PM, Richard Henderson wrote:
> On 05/17/2016 01:01 PM, Sergey Fedorov wrote:
>>> Sorry, I can't see reading ARMv6 ARM that 1-byte access can't be atomic.
>>> What
>>> I've found:
>>>
>>> B2.4.1 Normal memory attribute
>>> (snip)
>>> Shared Normal memory
>>>
>>>
On Tue, May 17, 2016 at 23:20:11 +0300, Sergey Fedorov wrote:
> On 17/05/16 23:04, Emilio G. Cota wrote:
(snip)
> > +/*
> > + * We might we tempted to use __atomic_test_and_set with __ATOMIC_ACQUIRE;
> > + * however, the documentation explicitly says that we should only pass
> > + * a boolean to it
From: Guillaume Delbergue
Signed-off-by: Guillaume Delbergue
[Rewritten. - Paolo]
Signed-off-by: Paolo Bonzini
[Emilio's additions: use TAS instead of atomic_xchg; emit acquire/release
barriers; call cpu_relax() while spinning; optimize for uncontended locks by
acquiring the lock with TAS ins
33 matches
Mail list logo
