[PATCH v6 0/1] MR7642: server: Use dlsym() to check for epoll_pwait2 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)~~ (already fixed.)

- 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.~~

--
  v6: server: Use dlsym() to check for epoll_pwait2 at run-time.

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

[PATCH v6 1/1] server: Use dlsym() to check for epoll_pwait2 at run-time.

[William Horvath]

From: William Horvath

This brings 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).
---
 configure.ac |  1 -
 server/fd.c  | 23 ++++++++++-------------
 2 files changed, 10 insertions(+), 14 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 bb0e90d99d0..a83f48007bb 100644
--- a/server/fd.c
+++ b/server/fd.c
@@ -100,6 +100,7 @@
 #include "ddk/wdm.h"

#if defined(HAVE_SYS_EPOLL_H) && defined(HAVE_EPOLL_CREATE)
+# include <dlfcn.h>
 # include <sys/epoll.h>
 # define USE_EPOLL
 #endif /* HAVE_SYS_EPOLL_H && HAVE_EPOLL_CREATE */
@@ -561,14 +562,13 @@ static inline void remove_epoll_user( struct fd *fd, int user )
     }
 }

+static int (*pepoll_pwait2)( int, struct epoll_event *, int, const struct timespec *, const sigset_t * );
+
 static inline void main_loop_epoll(void)
 {
     int i, ret, timeout;
     struct timespec ts;
     struct epoll_event events[128];
-#ifdef HAVE_EPOLL_PWAIT2
-    static int failed_epoll_pwait2 = 0;
-#endif

assert( POLLIN == EPOLLIN );
     assert( POLLOUT == EPOLLOUT );
@@ -577,6 +577,10 @@ static inline void main_loop_epoll(void)

if (epoll_fd == -1) return;

+    pepoll_pwait2 = dlsym( RTLD_DEFAULT, "epoll_pwait2" );
+    if (pepoll_pwait2 && pepoll_pwait2( -1, NULL, 0, 0, NULL ) == -1 && errno == ENOSYS)
+        pepoll_pwait2 = NULL;
+
     while (active_users)
     {
         timeout = get_next_timeout( &ts );
@@ -584,16 +588,9 @@ 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
-        if (!failed_epoll_pwait2)
-        {
-            ret = epoll_pwait2( epoll_fd, events, ARRAY_SIZE( events ), timeout == -1 ? NULL : &ts, NULL );
-            if (ret == -1 && errno == ENOSYS)
-                failed_epoll_pwait2 = 1;
-        }
-        if (failed_epoll_pwait2)
-#endif
-            ret = epoll_wait( epoll_fd, events, ARRAY_SIZE( events ), timeout );
+        /* use the highest resolution epoll available */
+        if (pepoll_pwait2) ret = pepoll_pwait2( epoll_fd, events, ARRAY_SIZE( events ), timeout == -1 ? NULL : &ts, NULL );
+        else ret = epoll_wait( epoll_fd, events, ARRAY_SIZE( events ), timeout );

set_current_time();

[William Horvath (＠whrvt)]

Moved the `ENOSYS` check to the same place `dlsym` is used.

[William Horvath (＠whrvt)]

> the way we usually do it is to just use syscall[...]

I'd personally be happier to try implementing the higher precision timeouts with something like a timerfd, before considering using a syscall wrapper around `epoll_pwait2`.

Of course it could be done, just define all the relevant syscall numbers, convert to a 64bit timespec, and do the `ENOSYS` test. It just feels like a dubious use of time which also adds some code bloat/complexity.

[William Horvath (＠whrvt)]

Closing, because I don't think this is really worth it anymore. Most people using Proton will have esync/fsync enabled where the side effects of the old `epoll_wait` behavior aren't much of an issue.

[PATCH v6 0/1] MR7642: server: Use dlsym() to check for epoll_pwait2 at run-time. - closed

[William Horvath (＠whrvt)]

This merge request was closed by William Horvath.