std: count all processor groups in available_parallelism on Windows#159511
std: count all processor groups in available_parallelism on Windows#159511valentynkit wants to merge 1 commit into
Conversation
On Windows 11 and Server 2022 a process spans every processor group by default, so GetSystemInfo undercounts a machine with more than 64 logical CPUs because it only reports the process's primary group. When the process is in more than one group, sum the active CPUs across all of them; otherwise keep using GetSystemInfo. The group count tells the two apart without an OS-version check. Add the matching Miri shims for the two new calls.
|
cc @rust-lang/miri |
rustbot
added
O-windows
|
|
| let group_count = this.deref_pointer_as(group_count, this.machine.layouts.u16)?; | ||
| this.write_scalar(Scalar::from_u16(2), &group_count)?; | ||
| this.write_scalar(Scalar::from_i32(1), dest)?; // TRUE | ||
| } |
There was a problem hiding this comment.
Each shim here should either
- be correct, in the sense that it checks all arguments for for every possible value either does the right thing or throws an "unsupported error"
- or be restricted to std like the shims at the bottom of this file (e.g.
SetThreadStackGuarantee)
These shims here ignore some of their arguments so they satisfy neither of these criteria.
| )?; | ||
| // Report more than one group so `available_parallelism` takes the | ||
| // `GetActiveProcessorCount(ALL_PROCESSOR_GROUPS)` path; a single-group | ||
| // affinity mask cannot hold the up-to-1024 CPUs the num-cpus test uses. |
There was a problem hiding this comment.
We could also just limit the number of CPUs we support on Windows to 64. It's entirely virtual anyway, Miri never really runs stuff in parallel.
FWIW, |
|
Cc @ChrisDenton |
4 participants
On Windows 11 and Server 2022 a process spans all processor groups by default, so
GetSystemInfoundercounts any machine with more than 64 CPUs: it only sees the primary group. Before Windows 11 the process is confined to that one group, so unconditionally switching toGetActiveProcessorCount(ALL_PROCESSOR_GROUPS)would overcount there instead. That's the Windows 10 problem you raised.GetProcessGroupAffinitygives the group count without an OS-version check: 1 before Windows 11, more than 1 once the process spans multiple groups. So gate on it, count every group when there's more than one, and fall back toGetSystemInfootherwise. A one-element buffer is enough: a multi-group process fails withERROR_INSUFFICIENT_BUFFERand writes the real count, every other case keeps the initial 1, so theBOOLis ignorable.Same call #114856 (klensy) proposed, now gated so it can't overcount on Windows 10.
Miri doesn't model processor groups, so the two new calls get shims reporting more than one group and
num_cpus. That runs the existing-Zmiri-num-cpus=1024test through the multi-group branch, which has to count rather than read a mask since a mask can't hold 1024 CPUs.A few cases stay imprecise:
std::thread::available_parallelismshould respect process affinity and job object limitations #143709.No hermetic test for the real >64-CPU path since it needs the hardware; the Miri test covers the branch. I can't build the Windows target or Miri on macOS, so CI is the real check.
Fixes #152389
r? @workingjubilee