Fall back to epoll_wait when epoll_pwait2 is unavailable at runtime - #23825
Conversation
HAVE_EPOLL_PWAIT2 only tells whether the libc exports the wrapper, which glibc does since 2.35 regardless of the running kernel. A PHP built on a kernel with epoll_pwait2 and run on one older than 5.11 gets ENOSYS from every Context::wait() call, which makes the epoll backend and thus the Auto backend unusable. The same happens under emulation layers that do not implement the syscall. Try epoll_pwait2 first and on ENOSYS or ENOTSUP switch the process to epoll_wait with a millisecond timeout, retrying the current call so the failure is never visible to the caller. The flag is process wide since kernel support is the same for every thread, and it is atomic so the first concurrent waits in a ZTS build do not race on it.
04d1b82 to
223dd37
Compare
…23825) HAVE_EPOLL_PWAIT2 only tells whether the libc exports the wrapper, which glibc does since 2.35 regardless of the running kernel. A PHP built on a kernel with epoll_pwait2 and run on one older than 5.11 gets ENOSYS from every Context::wait() call, which makes the epoll backend and thus the Auto backend unusable. The same happens under emulation layers that do not implement the syscall. Try epoll_pwait2 first and on ENOSYS or ENOTSUP switch the process to epoll_wait with a millisecond timeout, retrying the current call so the failure is never visible to the caller. The flag is process wide since kernel support is the same for every thread, and it is atomic so the first concurrent waits in a ZTS build do not race on it.
* PHP-8.6: Fall back to epoll_wait when epoll_pwait2 is unavailable at runtime (#23825)
* upstream/master: ext/bcmath: Clear the sign of BcMath\Number results that truncate to zero Use C11 atomics for epoll_pwait2_available Fix phpGH-23980: ZEND_ASSERT violation @ ZEND_INCLUDE_OR_EVAL (include/eval run with a pending exception) Fix phpGH-23842: skipLazyInitialization() copies unresolved constant defaults Fall back to epoll_wait when epoll_pwait2 is unavailable at runtime (php#23825) Fall back to epoll_wait when epoll_pwait2 is unavailable at runtime (php#23825) openssl: Fix memory leak by doing early salt validation ext/libxml: Keep SimpleXML children alive across reconstruction Fix phpGH-23741: pdo_dblib use-after-free of statement error state # Conflicts: # main/poll/poll_backend_epoll.c
| zend_atomic_bool_store_ex(&epoll_pwait2_available, false); | ||
| } | ||
| } | ||
| if (UNEXPECTED(!zend_atomic_bool_load_ex(&epoll_pwait2_available))) |
There was a problem hiding this comment.
This is not pretty: When pwait2 is unavailable at runtime, folks are paying for true branches, with one of them UNEXPECTED(). I would suggest making it a real else and then goto from line 194 into the else part, similarly to:
php-src/ext/random/zend_utils.c
Line 40 in 410e61d
There was a problem hiding this comment.
but this is for HAVE_EPOLL_PWAIT2 which means a case when it was compiled with such support but it runs on older kernel later. I was kind of assuming that such case should be UNEXPECTED...
There was a problem hiding this comment.
I was kind of assuming that such case should be UNEXPECTED...
My understanding is that UNEXPECTED is a very strong hint that the branch will not be taken. But that is not true: Once it is taken once, it will always be taken.
But I'm now realizing: This will likely always perform two atomic loads, even when pwait2 is actually available, since the compiler can't assume that the atomic doesn't change between the two conditions.
An explicit else (+ goto) has clearer semantics and should be faster in all cases. I wouldn't add any expected/unexpected hints, this is something that should be easily handled by the branch predictor.
There was a problem hiding this comment.
My understanding is that UNEXPECTED is a very strong hint that the branch will not be taken. But that is not true: Once it is taken once, it will always be taken.
This is correct. Generally, UNEXPECTED tends to move the code to the cold text section, use it generally only for error-handling code.
But I'm now realizing: This will likely always perform two atomic loads, even when pwait2 is actually available, since the compiler can't assume that the atomic doesn't change between the two conditions.
Also true
* PHP-8.6: (61 commits) zend_hrtime: use CLOCK_MONOTONIC instead of CLOCK_MONOTONIC_RAW (php#23790) sapi/cli: Fix built-in server truncating responses after a partial write Fix phpGH-24006: Skip gh18431.phpt when libzip lacks progress callbacks (php#24009) Updated to version 2026.5 (2026e) Fix phpGH-23896: Assertion failure in zend_call_function() after a throwing deprecation NEWS ext/gd: fix undefined behavior with GIFs with problematic LZW compression data Document missing deprecation entries for PHP 8.6 (php#23972) ext/tidy: Reject tidyNode use after the document is reparsed ext/bcmath: Clear the sign of BcMath\Number results that truncate to zero Fix phpGH-23980: ZEND_ASSERT violation @ ZEND_INCLUDE_OR_EVAL (include/eval run with a pending exception) Fix phpGH-23842: skipLazyInitialization() copies unresolved constant defaults Fall back to epoll_wait when epoll_pwait2 is unavailable at runtime (php#23825) openssl: Fix memory leak by doing early salt validation ext/libxml: Keep SimpleXML children alive across reconstruction Fix phpGH-23741: pdo_dblib use-after-free of statement error state NEWS Fix property hook escape analysis causing misoptimization Fix __isset escape analysis causing misoptimization Fix memory leak when closing a statement on a killed connection ...
4 participants
HAVE_EPOLL_PWAIT2only means the libc exports the wrapper, which glibc does since 2.35 regardless of the running kernel. A PHP built on a kernel withepoll_pwait2and run on one older than 5.11 getsENOSYSfrom everyContext::wait(), which makes the epoll backend and thusBackend::Autounusable. This is the normal shape for Docker images, distro packages and static builds, and it also happens under emulation layers that lack the syscall, as seen in #23478.Try
epoll_pwait2first and onENOSYSorENOTSUPswitch the process toepoll_wait, retrying the current call so the caller never sees the error. The flag is process-wide and atomic so ZTS builds do not race on it.The configure check is unchanged, so the plain link check keeps working when cross-compiling.
Alternative to #23478.