-
Notifications
You must be signed in to change notification settings - Fork 253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add cpu socket count in cpu.topology
#1497
feat: add cpu socket count in cpu.topology
#1497
Conversation
✅ Deploy Preview for kubernetes-sigs-nfd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
cpu.topology
cpu.topology
eef56a3
to
986aa41
Compare
/cc @marquiz |
986aa41
to
89b0e45
Compare
if we don't want to mount
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @AhmedGrati.
Instead of grepping /proc/cpuinfo (and mounting /proc from the host) we should get the information from sysfs. For example, iterate over cpu devices and read their topology/physical_package_id
. Number of unique ids is the number of sockets. I think we already loop over all cpus when checking for smt so we should be able to hook the new functionality there.
d362c6f
to
981b890
Compare
Thanks @marquiz @PiotrProkop. PTAL. |
/milestone v0.15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Two suggestions below
981b890
to
1be97df
Compare
Signed-off-by: AhmedGrati <[email protected]>
1be97df
to
ebb0836
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AhmedGrati 👍
We can add the label later if there is high demand for that
/assign @ArangoGutierrez |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
LGTM label has been added. Git tree hash: 20e7fc46e64c7cb531f6bb1afa85c19e88d6d15b
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AhmedGrati, ArangoGutierrez, marquiz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1497 +/- ##
==========================================
+ Coverage 30.66% 31.23% +0.56%
==========================================
Files 58 59 +1
Lines 7571 7671 +100
==========================================
+ Hits 2322 2396 +74
- Misses 4998 5025 +27
+ Partials 251 250 -1
|
Resolves #1383