-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[libc] Add implementation of getcpu syscall wrapper #150871
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
[libc] Add implementation of getcpu syscall wrapper #150871
Conversation
This patch adds the getcpu syscall wrapper. This has been supported in glibc since v2.29.
|
@llvm/pr-subscribers-libc Author: Aiden Grossman (boomanaiden154) ChangesThis patch adds the getcpu syscall wrapper. This has been supported in glibc since v2.29. Full diff: https://github.com/llvm/llvm-project/pull/150871.diff 8 Files Affected:
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 5e8278e586286..a1e93a133f262 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -36,6 +36,7 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.poll.poll
# sched.h entrypoints
+ libc.src.sched.getcpu
libc.src.sched.sched_get_priority_max
libc.src.sched.sched_get_priority_min
libc.src.sched.sched_getaffinity
diff --git a/libc/include/sched.yaml b/libc/include/sched.yaml
index 57871f524bf11..f14799ddf33fa 100644
--- a/libc/include/sched.yaml
+++ b/libc/include/sched.yaml
@@ -18,6 +18,13 @@ functions:
arguments:
- type: size_t
- type: const cpu_set_t *
+ - name: getcpu
+ standards:
+ - POSIX
+ return_type: int
+ arguments:
+ - type: unsigned int *
+ - type: unsigned int *
- name: sched_get_priority_max
standards:
- POSIX
diff --git a/libc/src/sched/CMakeLists.txt b/libc/src/sched/CMakeLists.txt
index e6c37d3b0433a..d1d1de0718c56 100644
--- a/libc/src/sched/CMakeLists.txt
+++ b/libc/src/sched/CMakeLists.txt
@@ -2,6 +2,13 @@ if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/${LIBC_TARGET_OS})
endif()
+add_entrypoint_object(
+ getcpu
+ ALIAS
+ DEPENDS
+ .${LIBC_TARGET_OS}.getcpu
+)
+
add_entrypoint_object(
sched_getaffinity
ALIAS
diff --git a/libc/src/sched/getcpu.h b/libc/src/sched/getcpu.h
new file mode 100644
index 0000000000000..ed3187a68fadc
--- /dev/null
+++ b/libc/src/sched/getcpu.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for getcpu ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_SCHED_GETCPU_H
+#define LLVM_LIBC_SRC_SCHED_GETCPU_H
+
+#include "src/__support/macros/config.h"
+#include <sched.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+int getcpu(unsigned int *cpu, unsigned int *node);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_SCHED_GETCPU_H
diff --git a/libc/src/sched/linux/CMakeLists.txt b/libc/src/sched/linux/CMakeLists.txt
index e690e76fd7483..66ebaeab43c21 100644
--- a/libc/src/sched/linux/CMakeLists.txt
+++ b/libc/src/sched/linux/CMakeLists.txt
@@ -1,3 +1,15 @@
+add_entrypoint_object(
+ getcpu
+ SRCS
+ getcpu.cpp
+ HDRS
+ ../getcpu.h
+ DEPENDS
+ libc.include.sched
+ libc.src.__support.OSUtil.osutil
+ libc.src.errno.errno
+)
+
add_entrypoint_object(
sched_getaffinity
SRCS
diff --git a/libc/src/sched/linux/getcpu.cpp b/libc/src/sched/linux/getcpu.cpp
new file mode 100644
index 0000000000000..2726d2daca2c5
--- /dev/null
+++ b/libc/src/sched/linux/getcpu.cpp
@@ -0,0 +1,30 @@
+//===-- Implementation of getcpu ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/sched/getcpu.h"
+
+#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
+#include "src/__support/common.h"
+#include "src/__support/libc_errno.h"
+#include "src/__support/macros/config.h"
+
+#include <sched.h>
+#include <sys/syscall.h> // For syscall numbers.
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(int, getcpu, (unsigned int *cpu, unsigned int *node)) {
+ int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_getcpu, cpu, node);
+ if (ret < 0) {
+ libc_errno = -ret;
+ return -1;
+ }
+ return 0;
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/test/src/sched/CMakeLists.txt b/libc/test/src/sched/CMakeLists.txt
index 9dda4ea16e101..a5b00049df2ca 100644
--- a/libc/test/src/sched/CMakeLists.txt
+++ b/libc/test/src/sched/CMakeLists.txt
@@ -40,6 +40,18 @@ add_libc_unittest(
libc.src.sched.sched_get_priority_max
)
+add_libc_unittest(
+ getcpu_test
+ SUITE
+ libc_sched_unittests
+ SRCS
+ getcpu_test.cpp
+ DEPENDS
+ libc.include.sched
+ libc.src.errno.errno
+ libc.src.sched.getcpu
+)
+
add_libc_unittest(
scheduler_test
SUITE
diff --git a/libc/test/src/sched/getcpu_test.cpp b/libc/test/src/sched/getcpu_test.cpp
new file mode 100644
index 0000000000000..5c3f531f76f72
--- /dev/null
+++ b/libc/test/src/sched/getcpu_test.cpp
@@ -0,0 +1,29 @@
+//===-- Unittests for getcpu ----------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/OSUtil/syscall.h"
+#include "src/__support/libc_errno.h"
+#include "src/sched/getcpu.h"
+#include "test/UnitTest/ErrnoSetterMatcher.h"
+
+#include <sched.h>
+
+TEST(LlvmLibcSchedGetCpuTest, SmokeTest) {
+ unsigned int current_cpu;
+ unsigned int current_node;
+ using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Succeeds;
+ ASSERT_THAT(LIBC_NAMESPACE::getcpu(¤t_cpu, ¤t_node), Succeeds(0));
+}
+
+TEST(LlvmLibcSchedGetCpuTest, BadPointer) {
+ unsigned int current_cpu;
+ unsigned int *current_node = reinterpret_cast<unsigned int *>(-1);
+ using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
+ ASSERT_THAT(LIBC_NAMESPACE::getcpu(¤t_cpu, current_node),
+ Fails(EFAULT));
+}
|
michaelrj-google
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine overall, needs some tweaks
libc/src/sched/linux/getcpu.cpp
Outdated
| #include "src/__support/libc_errno.h" | ||
| #include "src/__support/macros/config.h" | ||
|
|
||
| #include <sched.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this include is unnecessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. I figured since some of the other sched.h syscall wrappers included it, I would need it too.
Maybe an opportunity for cleanup for some of other wrappers...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, they definitely need some cleanup
libc/src/sched/getcpu.h
Outdated
| #define LLVM_LIBC_SRC_SCHED_GETCPU_H | ||
|
|
||
| #include "src/__support/macros/config.h" | ||
| #include <sched.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this include is unnecessary
libc/test/src/sched/getcpu_test.cpp
Outdated
| #include "src/sched/getcpu.h" | ||
| #include "test/UnitTest/ErrnoSetterMatcher.h" | ||
|
|
||
| #include <sched.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this include is unnecessary
libc/test/src/sched/getcpu_test.cpp
Outdated
|
|
||
| #include <sched.h> | ||
|
|
||
| TEST(LlvmLibcSchedGetCpuTest, SmokeTest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since these are setting errno, this should use ErrnoCheckingTest. See mmap_test.cpp for a good example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a ASSERT_ERRNO_SUCCESS in the successful call based on what I saw there. Not sure if there's anything else I need to change up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're using ASSERT_THAT you don't need the ASSERT_ERRNO_SUCCESS, but you do need to change these to TEST_F instead of TEST
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, missed the TEST_F. Updated.
michaelrj-google
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine overall, needs some tweaks
libc/test/src/sched/getcpu_test.cpp
Outdated
|
|
||
| #include <sched.h> | ||
|
|
||
| TEST(LlvmLibcSchedGetCpuTest, SmokeTest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're using ASSERT_THAT you don't need the ASSERT_ERRNO_SUCCESS, but you do need to change these to TEST_F instead of TEST
michaelrj-google
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one last change
libc/src/sched/linux/getcpu.cpp
Outdated
| namespace LIBC_NAMESPACE_DECL { | ||
|
|
||
| LLVM_LIBC_FUNCTION(int, getcpu, (unsigned int *cpu, unsigned int *node)) { | ||
| int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_getcpu, cpu, node); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the man page mentions that there's a third argument that should be null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yep. Looks like it's helpful to actually read the whole thing...
This patch adds the getcpu syscall wrapper. This has been supported in glibc since v2.29.