-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[libc][sched] Implement proxy headers for cpu_set_t, struct sched_param and sched_macros.
#126303
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
Changes from 20 commits
1a2fe13
ba8c535
c6a14a1
822bdb0
00b5943
8c990d2
926702c
02b5e6c
99ee89d
5a91cde
ece948f
c2ccccf
fd0ec52
32f9a0a
88fd93f
b34baef
de1eead
55849ca
7b48a0c
9a42b90
8baed07
9228a3b
0811fb1
23f30e9
cbcaf85
3f67c42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| //===-- Definition of macros from sched.h ---------------------------------===// | ||
| // | ||
| // 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_HDR_SCHED_MACROS_H | ||
| #define LLVM_LIBC_HDR_SCHED_MACROS_H | ||
|
|
||
| #ifdef LIBC_FULL_BUILD | ||
|
|
||
| #ifndef _SCHED_H | ||
| #define _SCHED_H 1 | ||
|
|
||
| #include "include/llvm-libc-macros/sched-macros.h" | ||
|
|
||
| #endif | ||
|
|
||
| #else // Overlay mode | ||
|
|
||
| #include <sched.h> | ||
|
|
||
| #endif // LLVM_LIBC_FULL_BUILD | ||
|
|
||
| #endif // LLVM_LIBC_HDR_SCHED_MACROS_H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| //===-- Proxy for cpu_set_t -----------------------------------------------===// | ||
| // | ||
| // 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_HDR_TYPES_CPU_SET_T_H | ||
| #define LLVM_LIBC_HDR_TYPES_CPU_SET_T_H | ||
|
|
||
| #ifdef LIBC_FULL_BUILD | ||
|
|
||
| #include "include/llvm-libc-types/cpu_set_t.h" | ||
|
|
||
| #else // Overlay mode | ||
|
|
||
| #include <sched.h> | ||
|
|
||
| #endif // LLVM_LIBC_FULL_BUILD | ||
|
|
||
| #endif // LLVM_LIBC_HDR_TYPES_CPU_SET_T_H | ||
nickdesaulniers marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| //===-- Proxy for struct sched_param --------------------------------------===// | ||
| // | ||
| // 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_HDR_TYPES_STRUCT_SCHED_PARAM_H | ||
| #define LLVM_LIBC_HDR_TYPES_STRUCT_SCHED_PARAM_H | ||
|
|
||
| #ifdef LIBC_FULL_BUILD | ||
|
|
||
| #include "include/llvm-libc-types/struct_sched_param.h" | ||
|
|
||
| #else // Overlay mode | ||
|
|
||
| #include <sched.h> | ||
|
|
||
| #endif // LLVM_LIBC_FULL_BUILD | ||
|
|
||
| #endif // LLVM_LIBC_HDR_TYPES_STRUCT_SCHED_PARAM_H |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ | |
| typedef struct { | ||
| // If a processor with more than 1024 CPUs is to be supported in future, | ||
| // we need to adjust the size of this array. | ||
| unsigned long __mask[128 / sizeof(unsigned long)]; | ||
| unsigned long __bits[128 / sizeof(unsigned long)]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's up with this change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so the glibc implementation of ref: https://elixir.bootlin.com/glibc/glibc-2.41/source/posix/bits/cpu-set.h#L38-L43
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right, and libc/include/llvm-libc-types/cpu_set_t.h should only be included in fullbuild mode, not overlay mode. |
||
| } cpu_set_t; | ||
|
|
||
| #endif // LLVM_LIBC_TYPES_CPU_SET_T_H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,20 +8,20 @@ | |
|
|
||
| #include "src/sched/sched_getcpucount.h" | ||
|
|
||
| #include "src/__support/common.h" | ||
| #include "src/__support/macros/config.h" | ||
| #include "src/__support/common.h" // LLVM_LIBC_FUNCTION | ||
| #include "src/__support/macros/config.h" // LIBC_NAMESPACE_DECL | ||
|
|
||
| #include <sched.h> | ||
| #include <stddef.h> | ||
| #include "hdr/types/cpu_set_t.h" | ||
| #include "hdr/types/size_t.h" | ||
|
|
||
| namespace LIBC_NAMESPACE_DECL { | ||
|
|
||
| LLVM_LIBC_FUNCTION(int, __sched_getcpucount, | ||
| (size_t cpuset_size, const cpu_set_t *mask)) { | ||
| int result = 0; | ||
| for (size_t i = 0; i < cpuset_size / sizeof(long); ++i) { | ||
| result += __builtin_popcountl(mask->__mask[i]); | ||
| } | ||
| for (size_t i = 0; i < cpuset_size / sizeof(long); ++i) | ||
| result += __builtin_popcountl(mask->__bits[i]); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change? Also, while you're here, mind removing the |
||
|
|
||
| return result; | ||
| } | ||
|
|
||
|
|
||
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 should not be in here. You are using this to suppress the system/glibc
sched.hdefinitions. We should investigate and make sure the header inclusions are separated in full build mode instead.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.
If for some reason, the system's
usr/include/sched.handlibc/hdr/sched_macros.hare included in the same translation unit, we need to do one of the following options:sched.hrelated headers to be complete enough so thatusr/include/sched.his not needed anymore in full build modeusr/include/sched.his not needed anymore in full build modeLet me know if you need help tracing back to how all of those inclusion conflicts are happening.
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.
guess i'll just have to work on this using the github workflow failures only lol. I tried:
followed by:
and i'm not able to reproduce these errors (on X86) so these are specific to aarch64 and i have no other way of testing for that architecture.
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 source
compiler-rt/lib/scudo/standalone/linux.cppthat is causing the errors usessched.h:llvm-project/compiler-rt/lib/scudo/standalone/linux.cpp
Line 23 in 3f67c42
in the function
getNumberOfCPUs:llvm-project/compiler-rt/lib/scudo/standalone/linux.cpp
Lines 164 to 171 in 3f67c42
which is declared in
common.hllvm-project/compiler-rt/lib/scudo/standalone/common.h
Lines 170 to 171 in 3f67c42
we already have definitions for
cpu_set_tandsched_getaffinitybut not forCPU_COUNT. I planned on first implementing the proxy headers forsched.hrelated definitions and then work on #124642 for the implementation of those macros but it seems there's a cyclic dependency within these issues. Sincesched.his not too big of a header, I think we should go with the 1st approach:@lntue How should I proceed? Should I complete #124642 prior to this, or implement CPU_COUNT within this PR only, which should complete the requirements for Scudo for now?
Uh oh!
There was an error while loading. Please reload this page.
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.
Oh, I missed it. we do have a definition for
CPU_COUNThere:llvm-project/libc/include/llvm-libc-macros/linux/sched-macros.h
Lines 26 to 27 in 3f67c42
that would mean we do have a complete enough
sched.himplementation so the bug shouldn't be because of some missing implementation (at least for now) which then means that the bug is due to the order of includes only?