Add get_online_cpus() utiilty function #39
Conversation
src/util.rs
Outdated
| cpu_range | ||
| .trim_end() | ||
| .parse::<u32>() | ||
| .expect("Unable to parse lone CPU number"), |
There was a problem hiding this comment.
How do you feel about returning an error here rather than panicking?
There was a problem hiding this comment.
That was the intent - fixing that.
src/util.rs
Outdated
| let end = cpu_range[index + 1..] | ||
| .trim_end() | ||
| .parse::<u32>() | ||
| .expect("ending CPU number"); |
| assert_eq!( | ||
| cpus, | ||
| (0..=1) | ||
| .chain(3..=3) |
There was a problem hiding this comment.
TIL about chain(), very cool!
There was a problem hiding this comment.
Thanks for this very needed change! LGTM.
I thought I had left a comment on some small improvement to how we test this. Ideally we would not rely on the filesystem (it might be wiped during test execution and cause the test to fail), which perhaps we could address by passing a memory buffer rather than an actual file (see how we do this for the kernel symbols parser). This is not a high priority of course, so I am happy to merge as-is and change this later on!
|
BTW I tested it locally on my x86 box and worked like a charm 😄 |
Attaching perf events can only be done for CPUs that are online - merely being present or possible isn't sufficient to allow such an event to be attached.
It's also possible for online CPU ranges to not be contiguous, due to NUMA topology or CPUs being offlined in some fashion.
So adding a utility function that provides a Vec of individual (not ranges of) online CPUs and using that instead of
num_possible_cpus(), which could be much larger than the number of online CPUs.Also updating code that tries to size certain data structures based on the number of online CPUs.
There are still places where the possible number of CPUs is still needed.
This is due to certain eBPF (libbpf) internals that have to allocate buffers based on the possible number of CPUs, rather than the online number - you'll only notice this on machines where they differ, which is pretty often for both physical and virtual machines.
Tested this one line of code on a host where the online number of CPUs was 4, but the possible was 8 to verify it works as expected (and fails when you try to use the online number instead).