Re: [PATCH 3/8] qemu-thread-win32: cleanup, fix, document QemuEvent

[David Hildenbrand <david@redhat.com>]

On 06.03.23 15:20, Paolo Bonzini wrote:
> On 3/6/23 14:31, David Hildenbrand wrote:
>>>
>>> -    smp_mb_acquire();
>>> -    if (value == EV_SET) {
>>> -        /* If there was a concurrent reset (or even reset+wait),
>>> -         * do nothing.  Otherwise change EV_SET->EV_FREE.
>>> -         */
>>> -        qatomic_or(&ev->value, EV_FREE);
>>> -    }
>>> +
>>
>> [had the same thought on patch #2]
>>
>> IIUC, the "read first" is an optimization to not unconditionally dirty
>> the cache-line. But I assume we don't particularly care about that
>> optimization on the reset path.
> 
> Thinking more about it, the intended usage of QemuEvent is either
> 
>       qemu_event_reset();
>       if (!check()) {
>            qemu_event_wait());
>       }
> 
> or
> 
>       if (!check()) {
>           qemu_event_reset();
>           if (!check()) {
>                qemu_event_wait());
>           }
>       }
> 
> If we don't care on the reset path we care much less on the wait path.
> Should I remove it and go straight to the cmpxchg, just for peace of mind?

Sounds reasonable to me at could simplify qemu_event_wait a bit.

-- 
Thanks,

David / dhildenb