From 3b9f16d40bc280236b99055781c131635671b5fa Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 22 Nov 2019 01:14:22 +0100 Subject: [PATCH 1/3] Fix getting affinity set on MUSL on Jetson TX2 The code in PAL_GetCurrentThreadAffinitySet relied on the fact that the number of processors reported as configured in the system is always larger than the maximum CPU index. However, it turns out that it is not true on some devices / distros. The Jetson TX2 reports CPUs 0, 3, 4 and 5 in the affinity mask and the 1 and 2 are never reported. GLIBC reports 6 as the number of configured CPUs, however MUSL reports just 4. The PAL_GetCurrentThreadAffinitySet was using the number of CPUs reported as configured as the upper bound for scanning affinity set, so on Jetson TX2, the affinity mask returned had just two bits set while there were 4 CPUs. That triggered an assert in the GCToOSInterface::Initialize. This change fixes that by reading the maximum CPU index from the /proc/cpuinfo. It falls back to using the number of processors configured when the /proc/cpuinfo is not available (on macOS, FreeBSD, ...) --- .../src/pal/src/include/pal/palinternal.h | 3 ++ src/coreclr/src/pal/src/misc/sysinfo.cpp | 49 ++++++++++++++++++- src/coreclr/src/pal/src/thread/thread.cpp | 4 +- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/pal/src/include/pal/palinternal.h b/src/coreclr/src/pal/src/include/pal/palinternal.h index 7fab86ee8d68c4..149c5aeafd8851 100644 --- a/src/coreclr/src/pal/src/include/pal/palinternal.h +++ b/src/coreclr/src/pal/src/include/pal/palinternal.h @@ -665,6 +665,9 @@ typedef enum _TimeConversionConstants bool ReadMemoryValueFromFile(const char* filename, uint64_t* val); +int +GetMaxCpuIndex(); + #ifdef __APPLE__ bool GetApplicationContainerFolder(PathCharString& buffer, const char *applicationGroupId, int applicationGroupIdLength); diff --git a/src/coreclr/src/pal/src/misc/sysinfo.cpp b/src/coreclr/src/pal/src/misc/sysinfo.cpp index fc10a3c89aa028..15e3d9c684e595 100644 --- a/src/coreclr/src/pal/src/misc/sysinfo.cpp +++ b/src/coreclr/src/pal/src/misc/sysinfo.cpp @@ -145,7 +145,7 @@ DWORD PALAPI PAL_GetLogicalCpuCountFromOS() { - static int nrcpus = -1; + static volatile int nrcpus = -1; if (nrcpus == -1) { @@ -167,6 +167,53 @@ PAL_GetLogicalCpuCountFromOS() return nrcpus; } +/*++ +Function: + GetMaxCpuIndex + +The GetMaxCpuIndex function returns maximum index of CPU available on the system. On some systems, it doesn't match +the PAL_GetTotalCpuCount() - 1, as the CPU indices may form a noncontinuous range. E.g. on Jetson TX2, there are +four CPUs available and their indices are 0, 3, 4, 5. While glibc returns 6 as the number of CPUs configured, +MUSL returns 4. +--*/ +int GetMaxCpuIndex() +{ + static volatile int maxCpuIndex = -1; + + if (maxCpuIndex == -1) + { + FILE* cpuInfoFile = fopen("/proc/cpuinfo", "r"); + if (cpuInfoFile != NULL) + { + char *line = nullptr; + size_t lineLen = 0; + + while (getline(&line, &lineLen, cpuInfoFile) != -1) + { + int cpuIndex; + int fieldsParsed = sscanf(line, "processor : %d", &cpuIndex); + + if ((fieldsParsed == 1) && (cpuIndex > maxCpuIndex)) + { + maxCpuIndex = cpuIndex; + } + } + + free(line); + fclose(cpuInfoFile); + } + + if (maxCpuIndex == -1) + { + // Reading the /proc/cpuinfo has failed, return the total CPU count - 1 as a fallback as we don't + // have anything better + maxCpuIndex = PAL_GetTotalCpuCount() - 1; + } + } + + return maxCpuIndex; +} + /*++ Function: GetSystemInfo diff --git a/src/coreclr/src/pal/src/thread/thread.cpp b/src/coreclr/src/pal/src/thread/thread.cpp index 4c4b6215451dc0..606037a56342e3 100644 --- a/src/coreclr/src/pal/src/thread/thread.cpp +++ b/src/coreclr/src/pal/src/thread/thread.cpp @@ -2935,10 +2935,10 @@ PAL_GetCurrentThreadAffinitySet(SIZE_T size, UINT_PTR* data) if (st == 0) { const SIZE_T BitsPerBitsetEntry = 8 * sizeof(UINT_PTR); - int nrcpus = PAL_GetTotalCpuCount(); + int maxCpuIndex = GetMaxCpuIndex(); // Get info for as much processors as it is possible to fit into the resulting set - SIZE_T remainingCount = std::min(size * BitsPerBitsetEntry, (SIZE_T)nrcpus); + SIZE_T remainingCount = std::min(size * BitsPerBitsetEntry, (SIZE_T)maxCpuIndex + 1); SIZE_T i = 0; while (remainingCount != 0) { From d2b289e49e1edb6bd3f09d01e05f546670660beb Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 22 Nov 2019 11:18:13 +0100 Subject: [PATCH 2/3] Reflect PR feedback Remove getting the max CPU index and use CPU_SETSIZE instead. Since the function is executed just once during startup, it is fine. --- src/coreclr/src/gc/unix/gcenv.unix.cpp | 2 +- .../src/pal/src/include/pal/palinternal.h | 3 -- src/coreclr/src/pal/src/misc/sysinfo.cpp | 49 +------------------ src/coreclr/src/pal/src/thread/thread.cpp | 3 +- 4 files changed, 3 insertions(+), 54 deletions(-) diff --git a/src/coreclr/src/gc/unix/gcenv.unix.cpp b/src/coreclr/src/gc/unix/gcenv.unix.cpp index e3f6131adec70f..4217bd88185fcc 100644 --- a/src/coreclr/src/gc/unix/gcenv.unix.cpp +++ b/src/coreclr/src/gc/unix/gcenv.unix.cpp @@ -330,7 +330,7 @@ bool GCToOSInterface::Initialize() if (st == 0) { - for (size_t i = 0; i < g_totalCpuCount; i++) + for (size_t i = 0; i < CPU_SETSIZE; i++) { if (CPU_ISSET(i, &cpuSet)) { diff --git a/src/coreclr/src/pal/src/include/pal/palinternal.h b/src/coreclr/src/pal/src/include/pal/palinternal.h index 149c5aeafd8851..7fab86ee8d68c4 100644 --- a/src/coreclr/src/pal/src/include/pal/palinternal.h +++ b/src/coreclr/src/pal/src/include/pal/palinternal.h @@ -665,9 +665,6 @@ typedef enum _TimeConversionConstants bool ReadMemoryValueFromFile(const char* filename, uint64_t* val); -int -GetMaxCpuIndex(); - #ifdef __APPLE__ bool GetApplicationContainerFolder(PathCharString& buffer, const char *applicationGroupId, int applicationGroupIdLength); diff --git a/src/coreclr/src/pal/src/misc/sysinfo.cpp b/src/coreclr/src/pal/src/misc/sysinfo.cpp index 15e3d9c684e595..fc10a3c89aa028 100644 --- a/src/coreclr/src/pal/src/misc/sysinfo.cpp +++ b/src/coreclr/src/pal/src/misc/sysinfo.cpp @@ -145,7 +145,7 @@ DWORD PALAPI PAL_GetLogicalCpuCountFromOS() { - static volatile int nrcpus = -1; + static int nrcpus = -1; if (nrcpus == -1) { @@ -167,53 +167,6 @@ PAL_GetLogicalCpuCountFromOS() return nrcpus; } -/*++ -Function: - GetMaxCpuIndex - -The GetMaxCpuIndex function returns maximum index of CPU available on the system. On some systems, it doesn't match -the PAL_GetTotalCpuCount() - 1, as the CPU indices may form a noncontinuous range. E.g. on Jetson TX2, there are -four CPUs available and their indices are 0, 3, 4, 5. While glibc returns 6 as the number of CPUs configured, -MUSL returns 4. ---*/ -int GetMaxCpuIndex() -{ - static volatile int maxCpuIndex = -1; - - if (maxCpuIndex == -1) - { - FILE* cpuInfoFile = fopen("/proc/cpuinfo", "r"); - if (cpuInfoFile != NULL) - { - char *line = nullptr; - size_t lineLen = 0; - - while (getline(&line, &lineLen, cpuInfoFile) != -1) - { - int cpuIndex; - int fieldsParsed = sscanf(line, "processor : %d", &cpuIndex); - - if ((fieldsParsed == 1) && (cpuIndex > maxCpuIndex)) - { - maxCpuIndex = cpuIndex; - } - } - - free(line); - fclose(cpuInfoFile); - } - - if (maxCpuIndex == -1) - { - // Reading the /proc/cpuinfo has failed, return the total CPU count - 1 as a fallback as we don't - // have anything better - maxCpuIndex = PAL_GetTotalCpuCount() - 1; - } - } - - return maxCpuIndex; -} - /*++ Function: GetSystemInfo diff --git a/src/coreclr/src/pal/src/thread/thread.cpp b/src/coreclr/src/pal/src/thread/thread.cpp index 606037a56342e3..6f42e94bd8aa0d 100644 --- a/src/coreclr/src/pal/src/thread/thread.cpp +++ b/src/coreclr/src/pal/src/thread/thread.cpp @@ -2935,10 +2935,9 @@ PAL_GetCurrentThreadAffinitySet(SIZE_T size, UINT_PTR* data) if (st == 0) { const SIZE_T BitsPerBitsetEntry = 8 * sizeof(UINT_PTR); - int maxCpuIndex = GetMaxCpuIndex(); // Get info for as much processors as it is possible to fit into the resulting set - SIZE_T remainingCount = std::min(size * BitsPerBitsetEntry, (SIZE_T)maxCpuIndex + 1); + SIZE_T remainingCount = std::min(size * BitsPerBitsetEntry, (SIZE_T)CPU_SETSIZE); SIZE_T i = 0; while (remainingCount != 0) { From f86d8e4187bd55bd77468d225026feb75a5d38f7 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 22 Nov 2019 13:55:54 +0100 Subject: [PATCH 3/3] Fix GCToOSInterface::GetProcessorForHeap too The loop was scanning only bits below the total processor count --- src/coreclr/src/gc/unix/gcenv.unix.cpp | 2 +- src/coreclr/src/vm/gcenv.os.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/gc/unix/gcenv.unix.cpp b/src/coreclr/src/gc/unix/gcenv.unix.cpp index 4217bd88185fcc..6bb0b94626bf1d 100644 --- a/src/coreclr/src/gc/unix/gcenv.unix.cpp +++ b/src/coreclr/src/gc/unix/gcenv.unix.cpp @@ -1210,7 +1210,7 @@ bool GCToOSInterface::GetProcessorForHeap(uint16_t heap_number, uint16_t* proc_n bool success = false; uint16_t availableProcNumber = 0; - for (size_t procNumber = 0; procNumber < g_totalCpuCount; procNumber++) + for (size_t procNumber = 0; procNumber < MAX_SUPPORTED_CPUS; procNumber++) { if (g_processAffinitySet.Contains(procNumber)) { diff --git a/src/coreclr/src/vm/gcenv.os.cpp b/src/coreclr/src/vm/gcenv.os.cpp index c224389426b1a3..d7e67485d64313 100644 --- a/src/coreclr/src/vm/gcenv.os.cpp +++ b/src/coreclr/src/vm/gcenv.os.cpp @@ -1007,7 +1007,7 @@ bool GCToOSInterface::GetProcessorForHeap(uint16_t heap_number, uint16_t* proc_n // Locate heap_number-th available processor uint16_t procIndex; size_t cnt = heap_number; - for (uint16_t i = 0; i < GCToOSInterface::GetTotalProcessorCount(); i++) + for (uint16_t i = 0; i < MAX_SUPPORTED_CPUS; i++) { if (g_processAffinitySet.Contains(i)) {