Re: [PATCH v4 00/11] Improve futex usage

[Peter Xu <peterx@redhat.com>]

On Tue, May 27, 2025 at 11:09:08AM +0900, Akihiko Odaki wrote:
> On 2025/05/26 23:48, Peter Xu wrote:
> > On Mon, May 26, 2025 at 02:29:10PM +0900, Akihiko Odaki wrote:
> > > Akihiko Odaki (11):
> > >        futex: Check value after qemu_futex_wait()
> > >        futex: Support Windows
> > >        qemu-thread: Remove qatomic_read() in qemu_event_set()
> > >        qemu-thread: Replace __linux__ with CONFIG_LINUX
> > >        qemu-thread: Avoid futex abstraction for non-Linux
> > >        qemu-thread: Use futex for QemuEvent on Windows
> > >        qemu-thread: Use futex if available for QemuLockCnt
> > >        migration: Replace QemuSemaphore with QemuEvent
> > >        migration/colo: Replace QemuSemaphore with QemuEvent
> > >        migration/postcopy: Replace QemuSemaphore with QemuEvent
> > 
> > In case it makes things easier.. I queued the three migration patches;
> > AFAIU they look like standalone to go even without prior patches, meanwhile
> > it shouldn't be an issue if they're queued in two pulls.
> > 
> > I am still not sure whether patch 1 is needed at all, but I'll leave that
> > to others to decide.
> 
> The migration patches shouldn't be applied before patches "futex: Check
> value after qemu_futex_wait()" and "qemu-thread: Avoid futex abstraction for
> non-Linux" as they fix QemuEvent. Merging migration patches earlier can
> trigger bugs just like one we faced with hw/display/apple-gfx*

I didn't see anything like it mentioned in either cover letter or the
apple-gfx patch.  Could you provide a pointer?

> 
> Regarding patch 1 ("futex: Check value after qemu_futex_wait()"), I can
> argue that we should have it to comply the generic "upstream-first"
> principle; the upstream (man page) says to make a loop, so the downstream
> (QEMU) should follow that until the upstream says otherwise. But I think
> it's a good idea to spend efforts to understand the reasoning behind the man
> page since it's so important that it affects not only QEMU but also any
> userspace program that uses libpthread (i.e., practically everything).

IMHO it's not how we define upstream/downstream, but maybe it's me who
missed something.

It's fine - as long as someone agrees with you (Paolo? :), it could likely
be that I was wrong and I just didn't realize that.

Thanks for the update anyway, I will drop the migration patches regardless.

-- 
Peter Xu