chcpu: bound the cpu list walk at the highest possible CPU - #634
Open
mmclinton wants to merge 2 commits into
Open
chcpu: bound the cpu list walk at the highest possible CPU#634mmclinton wants to merge 2 commits into
mmclinton wants to merge 2 commits into
Conversation
added 2 commits
August 24, 2026 20:43
`enabled_cpu_list` inlined the open, the read and the error path for one attribute name. A second cpu-list attribute is about to be read the same way, and duplicating eleven lines to change one string is the wrong shape. The helper takes `impl AsRef<Path>`, like every other accessor on `SysFSCpu`. `enabled_cpu_list` is its only caller for now, so behavior is unchanged.
A cpu-list range was bounded only by the integer type, so `CpuList::run` stepped through every index it named, one faccessat each, and printed one stderr line per index that did not exist. Measured at roughly 241,000 indices per second, which extrapolates to about five hours for `--enable 0-4294967295`; `--disable` took the same path and, as root, offlined the CPUs that do exist before grinding through the ones that do not. The walk now stops at the highest index in /sys/devices/system/cpu/possible. That mask is fixed during boot discovery, so no index above it can be brought online for the life of the boot, not even by hot-add, and `possible_cpus=` already pre-allocates slots for CPUs that are hot-added later: - https://docs.kernel.org/core-api/cpu_hotplug.html - https://www.kernel.org/doc/Documentation/ABI/testing/sysfs-devices-system-cpu Bounding there therefore cannot refuse an operation that could have succeeded. Only the walk is bounded, not the parse, so which argv are accepted is unchanged. Indices above the bound are reported one range at a time instead of one index at a time; a single index keeps its existing wording, so the common `chcpu -e 24` diagnostic is untouched. Exit status and stdout are unchanged for every argv: the collapsed remainder still counts as a failure, and an index above the bound never produced stdout to begin with. The bound is taken inside `walk_cpu_list` rather than passed to `run`, so no operation can be added that omits it: an omitted bound is the multi-hour walk the bound exists to prevent, with no compile error to catch it. Where the attribute cannot be read the walk stays unbounded, as before. Refusing the operation instead would let one missing optional attribute stop a CPU that does exist from being enabled. Three tests cover the bound; all walk only indices that are absent or already online, so they need no privileges and change no CPU state, and each skips itself where the bound cannot be read rather than walking unbounded. The two absent indices in the existing multi-element test are no longer adjacent, because a cpu-list coalesces touching ranges and each range is now reported once.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #620.
chcpuwalked every index in a cpu-list range, onefaccessatand one stderr line each, so a range as wide as the integer type took hours.The walk now stops at the highest index in
/sys/devices/system/cpu/possible, which is the bound I suggested on the issue.cpu_possible_maskis fixed during boot discovery, so nothing above it can be brought online for the life of the boot, not even by hot-add:So the bound cannot refuse an operation that would have worked. With it in place,
0-4294967295and0-18446744073709551615both return in about 2 ms.--enable,--disable,--configureand--deconfigureall walk the same cpu-list through one entry point, so all four are bounded on the same terms.Only the walk is bounded, not the parse, so which arguments are accepted is unchanged. Indices at or below the bound are walked exactly as before, which for
--disableas root means the CPUs that do exist are still offlined; they were in range and were asked for. The remainder above it is reported one range at a time rather than per index: on this box, wherepossibleis0-23,--enable 24-30becomes a single line instead of seven. A lone out-of-range index keeps its existing wording, sochcpu -e 24is untouched. Exit status and stdout are unchanged for every argument: the collapsed remainder still counts as a failure, and an index above the bound never produced stdout to begin with.Where
possibleis wider thanpresent, the absent CPUs below the bound are still reported one line each: withpossible=0-63andpresent=0-23,chcpu -e 20-70emits 40 per-index lines and then one collapsed range. Bounding atpresentinstead would avoid that, butpresentchanges on hot-add, so it can refuse an operation that would have worked.Where
possiblecannot be read or parsed the walk stays unbounded, as it was before, so on such a host the symptom in the issue comes back. The bound is best-effort, not a guarantee.