facebook / facebook/folly

CacheLocality::system() reports incorrect CPU count if offline CPUs present

Open
#1,219 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
30.5k
Forks
5.9k
PR merge metrics
No merged PRs in 30d

Description

### Summary

If one or more CPU cores are offline, then `CacheLocality::system()` mis-reports the number of CPU cores.

### Steps to reproduce

On a system with 24 logical cores (as reported by `lscpu -e`):

```
CPU NODE SOCKET CORE L1d:L1i:L2:L3 ONLINE MAXMHZ MINMHZ
0 0 0 0 0:0:0:0 yes 2400.0000 1200.0000
1 1 1 1 1:1:1:1 yes 2400.0000 1200.0000
2 0 0 2 2:2:2:0 yes 2400.0000 1200.0000
3 1 1 3 3:3:3:1 yes 2400.0000 1200.0000
4 0 0 4 4:4:4:0 yes 2400.0000 1200.0000
5 1 1 5 5:5:5:1 yes 2400.0000 1200.0000
6 0 0 6 6:6:6:0 yes 2400.0000 1200.0000
7 1 1 7 7:7:7:1 yes 2400.0000 1200.0000
8 0 0 8 8:8:8:0 yes 2400.0000 1200.0000
9 1 1 9 9:9:9:1 yes 2400.0000 1200.0000
10 0 0 10 10:10:10:0 yes 2400.0000 1200.0000
11 1 1 11 11:11:11:1 yes 2400.0000 1200.0000
12 0 0 0 0:0:0:0 yes 2400.0000 1200.0000
13 1 1 1 1:1:1:1 yes 2400.0000 1200.0000
14 0 0 2 2:2:2:0 yes 2400.0000 1200.0000
15 1 1 3 3:3:3:1 yes 2400.0000 1200.0000
16 0 0 4 4:4:4:0 yes 2400.0000 1200.0000
17 1 1 5 5:5:5:1 yes 2400.0000 1200.0000
18 0 0 6 6:6:6:0 yes 2400.0000 1200.0000
19 1 1 7 7:7:7:1 yes 2400.0000 1200.0000
20 0 0 8 8:8:8:0 yes 2400.0000 1200.0000
21 1 1 9 9:9:9:1 yes 2400.0000 1200.0000
22 0 0 10 10:10:10:0 yes 2400.0000 1200.0000
23 1 1 11 11:11:11:1 yes 2400.0000 1200.0000
```

1. Offline one CPU core (e.g. the 2nd):
```
chcpu --disable 1
```
2. Call `CacheLocality::system()` - e.g. via cache_locality_test
```
./cache_locality_test --gtest_filter=CacheLocality.LogSystem
```

Note that 24 cores are still reported:
```
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from CacheLocality
[ RUN ] CacheLocality.LogSystem
I0904 16:42:49.893321 37527 CacheLocalityTest.cpp:992] numCpus= 24
...
```

The same number is reported irrespective of how many CPU cores are still online - e.g. offlining half the CPU cores will still report "24".

Note the mis-counting has changed recently. Prior to the fix for #1208 (https://github.com/facebook/folly/commit/e545b9a9802107aa531e66d4996da7c19f53402c) it _would_ report the correct CPU count, however it suffered from the `std::vector` overrun reported in #1208, depending on which CPU cores were offline (if a CPU core number `C` was online such that `C` > `num_online_cpus` then you'd buffer overflow.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.