[PATCH v2 0/1] MR7642: server: Fall back to epoll_wait if epoll_pwait2 isn't available at run-time.

[William Horvath (＠whrvt)]

This fixes a real problem and improves behavior at the same time.
 - Fixes: hanging when trying to run Wine built with epoll_pwait2 headers available
  but epoll_pwait2 is missing in the run-time kernel (i.e. build 5.11->run 5.10)

 - Improvement: bring the improved timeout resolution to Wine builds
  which didn't have epoll_pwait2 at compile time, if the run-time kernel supports it
  (i.e. build 5.10->run 6.14)

This last point is especially important in my opinion, as it applies to official Proton builds.
Proton is (currently) built in Debian 11 with kernel 5.10, but SteamOS is a moving target with
a much newer kernel version being used to run these Wine builds.

These builds use the lower resolution timeouts if epoll_pwait2 is only checked for at compile-time,
when it can cheaply and easily be done at run-time instead.

Fixes: 87ca5db40e2c1b37423bdc25101a5c5e39e67e6f

An alternative to !7640 that kills two birds with one stone.

--
  v2: server: Fall back to epoll_wait if epoll_pwait2 isn't available at run-time.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7642

[PATCH v2 1/1] server: Fall back to epoll_wait if epoll_pwait2 isn't available at run-time.

[William Horvath]

From: William Horvath <william(a)horvath.blog>

This fixes a real problem and improves behavior at the same time.
 - Fixes: hanging when trying to run Wine built with epoll_pwait2 headers available
  but epoll_pwait2 is missing in the run-time kernel (i.e. build 5.11->run 5.10)

 - Improvement: bring the improved timeout resolution to Wine builds
  which didn't have epoll_pwait2 at compile time, if the run-time kernel supports it
  (i.e. build 5.10->run 6.14)

This last point is especially important in my opinion, as it applies to official Proton builds.
Proton is (currently) built in Debian 11 with kernel 5.10, but SteamOS is a moving target with
a much newer kernel version being used to run these Wine builds.

These builds use the lower resolution timeouts if epoll_pwait2 is only checked for at run-time,
when it can cheaply and easily be done at run-time instead.

Fixes: 87ca5db40e2c1b37423bdc25101a5c5e39e67e6f
---
 configure.ac |  1 -
 server/fd.c  | 20 +++++++++++++++-----
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 9e32070f610..05157e80d55 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2072,7 +2072,6 @@ AC_CHECK_FUNCS(\
         dladdr1 \
 	dlinfo \
 	epoll_create \
-	epoll_pwait2 \
 	fstatfs \
 	futimens \
 	futimes \
diff --git a/server/fd.c b/server/fd.c
index e1b969a0439..664c830cdf2 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -561,6 +561,20 @@ static inline void remove_epoll_user( struct fd *fd, int user )
     }
 }
 
+/* dispatch to the highest resolution epoll available */
+static inline int do_epoll_wait( struct epoll_event *events, int maxevents, const struct timespec *timeout_ts, int timeout_ms )
+{
+    static BOOL have_pwait2 = TRUE;
+    if (have_pwait2)
+    {
+        int ret;
+        ret = syscall( 441 /* __NR_epoll_pwait2 */, epoll_fd, events, maxevents, timeout_ts, NULL, sizeof(unsigned long) );
+        if (ret == -1 && errno == ENOSYS) have_pwait2 = FALSE;
+        else return ret;
+    }
+    return epoll_wait( epoll_fd, events, maxevents, timeout_ms );
+}
+
 static inline void main_loop_epoll(void)
 {
     int i, ret, timeout;
@@ -581,11 +595,7 @@ static inline void main_loop_epoll(void)
         if (!active_users) break;  /* last user removed by a timeout */
         if (epoll_fd == -1) break;  /* an error occurred with epoll */
 
-#ifdef HAVE_EPOLL_PWAIT2
-        ret = epoll_pwait2( epoll_fd, events, ARRAY_SIZE( events ), timeout == -1 ? NULL : &ts, NULL );
-#else
-        ret = epoll_wait( epoll_fd, events, ARRAY_SIZE( events ), timeout );
-#endif
+        ret = do_epoll_wait( events, ARRAY_SIZE( events ), timeout == -1 ? NULL : &ts, timeout );
 
         set_current_time();
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7642

[PATCH v2 0/1] MR7642: Draft: server: Fall back to epoll_wait if epoll_pwait2 isn't available at run-time.

[Alfred Agrell (＠Alcaro)]

Alfred Agrell (@Alcaro) commented about server/fd.c:
> }
>  }
>
> +/* dispatch to the highest resolution epoll available */
> +static inline int do_epoll_wait( struct epoll_event *events, int maxevents, const struct timespec *timeout_ts, int timeout_ms )
> +{
> +    static BOOL have_pwait2 = TRUE;
> +    if (have_pwait2)
> +    {
> +        int ret;
> +        ret = syscall( 441 /* __NR_epoll_pwait2 */, epoll_fd, events, maxevents, timeout_ts, NULL, sizeof(unsigned long) );

__NR_epoll_pwait2 is 441 on most archs, but it's not guaranteed; there are a few cases (like x32 abi) where syscalls don't start from zero at all.

I'm not sure if Wine supports any such archs or ABIs, but it's worth checking, at least.

That last arg should be sizeof(sigset_t), but I don't think it matters if the actual signal set is null.

I have no opinion on your approach vs mine; mine's a little simpler, but yours is more comprehensive. I'm fine with both.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7642#note_98739

[PATCH v2 0/1] MR7642: Draft: server: Fall back to epoll_wait if epoll_pwait2 isn't available at run-time.

[William Horvath (＠whrvt)]

> should be sizeof(sigset_t)

I was reading the glibc source and verifying strace output, `1*sizeof(unsigned long)` is correct in this case for NULL sigset.

But yes, hardcoding the syscall number is a bad idea, and the issue with the linux-32 tests is that epoll_pwait2 only takes 64-bit timespecs (which the glibc wrapper automatically converts). I'll undraft it when I revise the approach.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7642#note_98740