-
Notifications
You must be signed in to change notification settings - Fork 184
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
pci: report NUMA node locality for the PCI devices #225
Conversation
(Implicitely) depends on #224 |
f11d823
to
783de03
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.
@fromanirh I'm happy with this, all looks good to me. Are we good to merge this one?
pkg/pci/pci.go
Outdated
@@ -39,6 +40,9 @@ type Device struct { | |||
Subclass *pcidb.Subclass `json:"subclass"` | |||
// optional programming interface | |||
ProgrammingInterface *pcidb.ProgrammingInterface `json:"programming_interface"` | |||
// Topology node that the graphics card is affined to. Will be nil if the |
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.
s/graphics card/PCI device/
:)
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.
yeah, copy/paste strikes again :)
// NewWithContext returns a pointer to an Info struct that contains information about | ||
// the NUMA topology on the host system. Use this function when you want to consume | ||
// the topology package from another package (e.g. pci, gpu) | ||
func NewWithContext(ctx *context.Context) (*Info, error) { |
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.
👍
hi Jay! We can merge this PR (pending me addressing your comments :) ) with the caveat that |
We want to expose the NUMA locality of the PCI devices, much like we already do for GPU devices. This is very important for high-end, high-performance devices (NICs, storage controllers). To enable testing, we update the snapshot with another one captured with more PCI device informations. See: jaypipes#224 Signed-off-by: Francesco Romani <[email protected]>
783de03
to
6a1e32f
Compare
And btw, after some trial/errors, #224 reached a form I'm comfortable with, and requires just a minor followup now. |
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.
👍 nice work Francesco
We want to expose the NUMA locality of the PCI devices,
much like we already do for GPU devices.
This is very important for high-end, high-performance devices
(NICs, storage controllers).
To enable testing, we update the snapshot with another one
captured with more PCI device informations.
See: #224
Signed-off-by: Francesco Romani [email protected]