linux: treat a negative topology id as unavailable, not a parse error#421
Open
CodersAcademy006 wants to merge 1 commit into
Open
linux: treat a negative topology id as unavailable, not a parse error#421CodersAcademy006 wants to merge 1 commit into
CodersAcademy006 wants to merge 1 commit into
Conversation
The kernel exposes core_id and physical_package_id as -1 when it has no
topology information for a processor. Both were parsed with uint32_parser,
which rejects any non-digit lead byte, so every such processor produced an
error-level log claiming the sysfs file was malformed:
Error in cpuinfo: failed to parse file
/sys/devices/system/cpu/cpu0/topology/core_id: "-1" is not an unsigned number
-1 is a sentinel, not corruption. Parse it as "value unavailable" via a
topology_id_parser wrapper and log it at debug level. The return value is
unchanged (false, the id is genuinely unavailable), so no caller behavior
changes; only the false alarm goes away.
Observed on a native riscv64 board (SiFive U74-class, kernel 5.10.113),
which reports core_id -1 on all four cores. physical_package_id parsed fine
there, but is exposed to the same sentinel and is covered for consistency.
Also fixes uint32_parser's empty-file branch, which hardcoded
KERNEL_MAX_FILENAME while the parser is shared by six call sites, so an
empty core_id file would have blamed the wrong path.
Test Plan: compiled src/linux/processors.c into a harness driving the real
static parsers.
clang -DCPUINFO_LOG_LEVEL=5 -I src -I include -o t t.c src/log.c
before: core_id "-1" -> ok=false + ERROR "is not an unsigned number"
after : core_id "-1" -> ok=false + DEBUG "reports no topology information"
core_id "0" -> ok=true value=0
core_id "3" -> ok=true value=3
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
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.
The kernel exposes
core_idandphysical_package_idas-1when it has no topology information for a processor. Both are parsed withuint32_parser, which rejects any non-digit lead byte, so every such processor produces an error-level log claiming the sysfs file is malformed.Observed on a native riscv64 board (SiFive U74-class, kernel 5.10.113), which reports
core_id = -1on all four cores:-1is a documented sentinel meaning "not populated", not corruption, so this is a false alarm rather than a real parse failure.Root cause
parse_number(src/linux/processors.c) stops at the first non-digit. For-1it stops immediately on the-, leavingparsed_end == text_start, whichuint32_parserreports as"is not an unsigned number"at error level.The change
Add a
topology_id_parserwrapper used by the two topology getters. A topology id is never legitimately negative, so a leading-is reported as "value unavailable" at debug level instead. The return value is unchanged (false, since the id genuinely is unavailable), so no caller behavior changes. Only the false alarm goes away.uint32_parsercontinues to backkernel_maxand the three frequency getters, where a negative value would still be a genuine error.physical_package_idparsed fine on the board above, but is exposed to the same sentinel through the same kernel mechanism, so it is covered for consistency.Also fixed
The empty-file branch in
uint32_parserhardcodedKERNEL_MAX_FILENAMEeven though the parser is shared by six call sites, so an emptycore_idfile would have blamed/sys/devices/system/cpu/kernel_max. It now reports thefilenameit was given.Test plan
Compiled the real
src/linux/processors.cinto a harness driving its actual static parsers (stubbing only the sysfs reader), so this exercises the shipped code rather than a copy:Before, reproducing the board output exactly:
After: