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

[David Hildenbrand]

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