Re: [PATCH 2/8] qemu-thread-posix: cleanup, fix, document QemuEvent

[David Hildenbrand]

> -             * Leave the event reset and tell qemu_event_set that there
> -             * are waiters.  No need to retry, because there cannot be
> -             * a concurrent busy->free transition.  After the CAS, the
> -             * event will be either set or busy.
> +             * Leave the event reset and tell qemu_event_set that there are
> +             * waiters.  No need to retry, because there cannot be a concurrent
> +             * busy->free transition.  After the CAS, the event will be either
> +             * set or busy.
> +             *
> +             * Neither the load nor the store of this cmpxchg have particular
> +             * ordering requirements.  The reasoning for the load is the same
> +             * as qatomic_read() above; while moving the store earlier can only
> +             * cause qemu_event_set() to issue _more_ wakeups.

IIUC, the qatomic_read(&ev->value) is mostly an optimization then, to 
not do an unconditional qatomic_cmpxchg(). That's why we don't care 
about the order in particular.

>                */
>               if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
>                   return;
>               }
>           }
> +
> +        /*
> +         * This is the final check for a concurrent set, so it does need
> +         * a smp_mb() pairing with the second barrier of qemu_event_set().
> +         * The barrier is inside the FUTEX_WAIT system call.
> +         */
>           qemu_futex_wait(ev, EV_BUSY);
>       }
>   }


Skipping back and forth between the Linux and QEMU memory model is a pain :D

Reviewed-by: David Hildenbrand <david@redhat.com>

--
Thanks,

David / dhildenb