Skip to content

[libc] Implement pwait2 using pwait #99781

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 20, 2024

Conversation

mikhailramalho
Copy link
Member

This patch implements pwait2 using pwait. The implementation is an approximation of pwait2, since pwait only only supports timeouts in milliseconds, not nanoseconds, as required by pwait2.

This patch implements pwait2 using pwait. The implementation is an
approximation of pwait2, since pwait only only supports timeouts in
milliseconds, not nanoseconds, as required by pwait2.
@llvmbot
Copy link
Member

llvmbot commented Jul 20, 2024

@llvm/pr-subscribers-libc

Author: Mikhail R. Gadelha (mikhailramalho)

Changes

This patch implements pwait2 using pwait. The implementation is an approximation of pwait2, since pwait only only supports timeouts in milliseconds, not nanoseconds, as required by pwait2.


Full diff: https://github.com/llvm/llvm-project/pull/99781.diff

1 Files Affected:

  • (modified) libc/src/sys/epoll/linux/epoll_pwait2.cpp (+12)
diff --git a/libc/src/sys/epoll/linux/epoll_pwait2.cpp b/libc/src/sys/epoll/linux/epoll_pwait2.cpp
index 14b419399fe9b..4123157d29fff 100644
--- a/libc/src/sys/epoll/linux/epoll_pwait2.cpp
+++ b/libc/src/sys/epoll/linux/epoll_pwait2.cpp
@@ -25,10 +25,22 @@ namespace LIBC_NAMESPACE_DECL {
 LLVM_LIBC_FUNCTION(int, epoll_pwait2,
                    (int epfd, struct epoll_event *events, int maxevents,
                     const struct timespec *timeout, const sigset_t *sigmask)) {
+#ifdef SYS_epoll_pwait2
   int ret = LIBC_NAMESPACE::syscall_impl<int>(
       SYS_epoll_pwait2, epfd, reinterpret_cast<long>(events), maxevents,
       reinterpret_cast<long>(timeout), reinterpret_cast<long>(sigmask),
       NSIG / 8);
+#elif defined(SYS_epoll_pwait)
+  // Convert nanoseconds to milliseconds, rounding up if there are remaining
+  // nanoseconds
+  long timeout_ms = static_cast<long>(timeout->tv_sec * 1000 +
+                                      (timeout->tv_nsec + 999999) / 1000000);
+  int ret = LIBC_NAMESPACE::syscall_impl<int>(
+      SYS_epoll_pwait, epfd, reinterpret_cast<long>(events), maxevents,
+      timeout_ms, reinterpret_cast<long>(sigmask), NSIG / 8);
+#else
+#error "epoll_pwait and epoll_pwait2 syscalls not available."
+#endif
 
   // A negative return value indicates an error with the magnitude of the
   // value being the error code.

@mikhailramalho mikhailramalho merged commit 569814e into llvm:main Jul 20, 2024
8 checks passed
@mikhailramalho mikhailramalho deleted the pwait2-using-pwait branch July 20, 2024 20:27
Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good idea. I would rather just not provide epoll_pwait2 on platforms that don't support it.

michaelrj-google added a commit to michaelrj-google/llvm-project that referenced this pull request Jul 22, 2024
This patch reverts llvm#99781 and part of llvm#99771 since `epoll_pwait2` is not
in fact available on all supported systems. It is my opinion that we
shouldn't provide a version of a function that doesn't perform as
expected, which is why this revert needs to happen.

The `epoll_pwait2` function can be reenabled when we have a way to check
if it is available on the target system, tracking bug for that is llvm#80060
michaelrj-google added a commit that referenced this pull request Jul 22, 2024
This patch reverts #99781 and part of #99771 since `epoll_pwait2` is not
in fact available on all supported systems. It is my opinion that we
shouldn't provide a version of a function that doesn't perform as
expected, which is why this revert needs to happen.

The `epoll_pwait2` function can be reenabled when we have a way to check
if it is available on the target system, tracking bug for that is #80060
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
This patch implements pwait2 using pwait. The implementation is an
approximation of pwait2, since pwait only only supports timeouts in
milliseconds, not nanoseconds, as required by pwait2.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251459
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
This patch reverts #99781 and part of #99771 since `epoll_pwait2` is not
in fact available on all supported systems. It is my opinion that we
shouldn't provide a version of a function that doesn't perform as
expected, which is why this revert needs to happen.

The `epoll_pwait2` function can be reenabled when we have a way to check
if it is available on the target system, tracking bug for that is #80060
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants