Read core and physical package ID of the CPUs that are online #362
Conversation
kishen-v
force-pushed
the
determine-cpu-online-sts
branch
from
6d1e9a9 to
151c79a
Compare
|
LGTM any chance to add a test or two to exercise this code? |
kishen-v
force-pushed
the
determine-cpu-online-sts
branch
4 times, most recently
from
d2d4d98 to
524a3b3
Compare
|
Hi @ffromani, I've added a couple of tests to check if the warning is being raised in the case of missing files (core_id/physical_package_id) in the topology directory. Could you PTAL when time permits? Thanks! |
| @@ -64,6 +65,10 @@ func processorsGet(ctx *context.Context) []*Processor { | |||
| continue | |||
| } | |||
|
|
|||
| onlineFilePath := filepath.Join(paths.SysDevicesSystemCPU, fmt.Sprintf("cpu%d", lpID), onlineFile) | |||
| if util.SafeIntFromFile(ctx, onlineFilePath) == 0 { | |||
There was a problem hiding this comment.
this will return -1 if the file does not exist... perhaps it's worth changing this to:
if util.SafeIntFromFile(ctx, onlineFilePath) != 1 {in order to account for when there are problems locating the /online file?
There was a problem hiding this comment.
I agree. Addressed the comment.
Thanks!
There was a problem hiding this comment.
It was observed that this change didn't fit well for the ARM architecture, as the online file does not exist under the cpuX directory. This lead to the mismatch in the expected vs observed tests results causing a breaking change.
The online file exists for the amd64/ppc64le architecture where, the corresponding files are in the topology directory may/may not exist based on the bit set in the online file. I have made changes to account only if the cpu is offline, before the files under the topology directory are checked. This doesn't seem to affect the arm64 architecture as the -1 returned from the caller doesn't affect the rest of the code execution.
Thanks!
kishen-v
force-pushed
the
determine-cpu-online-sts
branch
2 times, most recently
from
09df61e to
1b877ca
Compare
kishen-v
force-pushed
the
determine-cpu-online-sts
branch
from
1b877ca to
d63f40c
Compare
Add tests for files in topology directory for offline CPUs Signed-off-by: Kishen V <kishen.viswanathan@ibm.com>
kishen-v
force-pushed
the
determine-cpu-online-sts
branch
from
d63f40c to
e576a6a
Compare
Fixes: #361
Added check to fetch the physical_package_id/core_id after determining if the CPU is online.