Re: [PATCH v5 00/13] Improve futex usage

2025-06-06 Thread Akihiko Odaki

On 2025/06/07 5:38, Paolo Bonzini wrote:

On Fri, Jun 6, 2025 at 11:46 AM Akihiko Odaki  wrote:

This conditional is unnecessary; docs/about/build-platforms.rst says we
only supports MinGW.

I failed to catch this problem because I ran MinGW on Windows, which is
case-insensitive. Since it is case-insensitive, the lowercase name will
work for the OS too.


Possibly, but better safe than sorry... The official name is the uppercase one.


There are already two libraries that are referenced with lowercase names 
while they have uppercase characters in Windows SDK:


MinGW  | Windows SDK
---+
ws2_32 | WS2_32
winmm  | WinMM

So there is no gain by trying out the name of the synchronization 
library in Windows SDK; the build will break anyway if a build platform 
requires case-sensitive matches with Windows SDK names.


I also think it's better to keep the library references consistent; 
there is no reason to check the case-preserved name of a library while 
not checking the one for another and vice versa. In this sense, we 
should consistently check the case-preserved names or consistently avoid 
checking them.


Regards,
Akihiko Odaki



Re: [PATCH v5 00/13] Improve futex usage

2025-06-06 Thread Paolo Bonzini
On Fri, Jun 6, 2025 at 11:46 AM Akihiko Odaki  wrote:
> This conditional is unnecessary; docs/about/build-platforms.rst says we
> only supports MinGW.
>
> I failed to catch this problem because I ran MinGW on Windows, which is
> case-insensitive. Since it is case-insensitive, the lowercase name will
> work for the OS too.

Possibly, but better safe than sorry... The official name is the uppercase one.

Paolo




Re: [PATCH v5 00/13] Improve futex usage

2025-06-06 Thread Akihiko Odaki

On 2025/06/05 22:24, Paolo Bonzini wrote:

From: Akihiko Odaki 


In a recent discussion, Phil Dennis-Jordan pointed out a quirk in
QemuEvent destruction due to futex-like abstraction, which prevented
the usage of QemuEvent in new and existing code[1]. With some more
thoughts after this discussion, I also found other problem and room
of improvement in futex usage. Here is a stack of patches to resolve
them.


Thanks.  I haven't had the time to go through the final two patches,
so I took the current Linux code unmodified and added the non-Linux
changes on top.  But I have kept the delta locally and will get back
to it.

I also had to squash this in for CI to pass:

diff --git a/meson.build b/meson.build
index 20e8f37e6e7..34729c2a3dd 100644
--- a/meson.build
+++ b/meson.build
@@ -843,7 +843,12 @@ if host_os == 'windows'
midl = find_program('midl', required: false)
widl = find_program('widl', required: false)
pathcch = cc.find_library('pathcch')
-  synchronization = cc.find_library('Synchronization')
+  synchronization = cc.find_library('Synchronization', required: false)
+  if not synchronization.found()
+# The library name is lowercase on mingw
+synchronization = cc.find_library('synchronization', required: true)
+  endif
+


This conditional is unnecessary; docs/about/build-platforms.rst says we 
only supports MinGW.


I failed to catch this problem because I ran MinGW on Windows, which is 
case-insensitive. Since it is case-insensitive, the lowercase name will 
work for the OS too.


Regards,
Akihiko Odaki



Re: [PATCH v5 00/13] Improve futex usage

2025-06-05 Thread Paolo Bonzini
From: Akihiko Odaki 

> In a recent discussion, Phil Dennis-Jordan pointed out a quirk in
> QemuEvent destruction due to futex-like abstraction, which prevented
> the usage of QemuEvent in new and existing code[1]. With some more
> thoughts after this discussion, I also found other problem and room
> of improvement in futex usage. Here is a stack of patches to resolve
> them.

Thanks.  I haven't had the time to go through the final two patches,
so I took the current Linux code unmodified and added the non-Linux
changes on top.  But I have kept the delta locally and will get back
to it.

I also had to squash this in for CI to pass:

diff --git a/meson.build b/meson.build
index 20e8f37e6e7..34729c2a3dd 100644
--- a/meson.build
+++ b/meson.build
@@ -843,7 +843,12 @@ if host_os == 'windows'
   midl = find_program('midl', required: false)
   widl = find_program('widl', required: false)
   pathcch = cc.find_library('pathcch')
-  synchronization = cc.find_library('Synchronization')
+  synchronization = cc.find_library('Synchronization', required: false)
+  if not synchronization.found()
+# The library name is lowercase on mingw
+synchronization = cc.find_library('synchronization', required: true)
+  endif
+
   socket = cc.find_library('ws2_32')
   winmm = cc.find_library('winmm')

Paolo