snapshot: gather full pci tree, handling bridges #224
Merged
Conversation
This file contains 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
|
Ready for design review; docstrings are missing, and I need to review unit tests. hence the WIP. Conflicts with #223 |
ffromani
force-pushed
the
snapshot-pci-tree
branch
from
3d6ae01 to
bf5afbd
Compare
ffromani
changed the title
ffromani
added a commit
to ffromani/ghw
that referenced
this pull request
Feb 8, 2021
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]>
This was referenced Feb 8, 2021
ffromani
force-pushed
the
snapshot-pci-tree
branch
from
bf5afbd to
1a91be6
Compare
ffromani
commented
Feb 9, 2021
ffromani
commented
Feb 9, 2021
ffromani
force-pushed
the
snapshot-pci-tree
branch
3 times, most recently
from
aba6ca6 to
7ec16bd
Compare
ffromani
added a commit
to ffromani/ghw
that referenced
this pull request
Feb 11, 2021
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]>
ffromani
force-pushed
the
snapshot-pci-tree
branch
from
7ec16bd to
1779337
Compare
|
clarified (again) the commit message |
|
still not completely happy. Let me evaluate another approach, will update ASAP. |
ffromani
force-pushed
the
snapshot-pci-tree
branch
from
1779337 to
9b19ce6
Compare
|
I'm still testing 9b19ce6 and I'm happier with this approach so far. |
ffromani
force-pushed
the
snapshot-pci-tree
branch
2 times, most recently
from
fa85ffd to
f08ab4b
Compare
ffromani
added a commit
to ffromani/ghw
that referenced
this pull request
Feb 23, 2021
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]>
ffromani
force-pushed
the
snapshot-pci-tree
branch
from
f08ab4b to
7a30a06
Compare
|
rebased on top of #227 . I think this PR is now good enough. |
ffromani
force-pushed
the
snapshot-pci-tree
branch
from
7a30a06 to
341af48
Compare
|
rebased again. No code changes. |
jaypipes
reviewed
Feb 25, 2021
In order to maximize the code reuse, we move the code which handles PCI addresses to its own subpackage. This is driven by a upcoming patch, which wants to parse PCI addresses from strings, without consuming the pci package, to avoid circular imports. Only trivial code movement + needed renames e.g.: AddressFromString() -> address.FromString() Signed-off-by: Francesco Romani <[email protected]>
let's add Warn()ings when PCI scan fails, to improve the troubleshooting process. Signed-off-by: Francesco Romani <[email protected]>
Modern (x86_64) systems have multiple PCI buses.
For ghw needs, considering the abstraction provided by the linux kernel,
there is no practical difference from PCI-express and PCI to date,
so we will call them all "PCI".
PCI-to-PCI (see initial note about PCI express) are common, and their
topology depends on the CPU, mainboard and system manufacturer choice.
The linux kernel does not flatten the PCI tree in sysfs.
This choice provides the closest representation to reality, and
unfortunately is also easy to overlook
when exploring simple PCI device trees, such as ones found on laptops,
but becomes evident when exploring more complex trees, like the ones
we can see on servers, even low end.
This means, in case of PCI bridges, we will have
entries inside the directory which represent a bus which
will be both devices and root of subtrees.
In other words, these entries will contain BOTH subdirectories
to represent the devices attached to that bridge, and the files
which describe the bridge device per se.
For ghw, the net effect is we cannot just scan the entries
in `/sys/bus/pci/devices` and be done with it; we need to detect
the entries which represent PCI bridges, and scan those recursively.
Otherwise, we miss to gather the data pertaining to these devices.
So we need a smarter function that does this traversal, moving away
from the nice static list of paths we previously had.
Without this patch (example taken from dev laptop):
{host bridge}
-[0000:00]-+-00.0
+-02.0
+-14.0
+-14.2
+-16.0
+-16.3
+-1c.0 {PCI bridge}
+-1c.2 {PCI bridge}
+-1d.0 {PCI bridge}
+-1f.0
+-1f.2
+-1f.3
+-1f.4
\-1f.6
With this patch (example taken from dev laptop):
{host bridge}
-[0000:00]-+-00.0
+-02.0
+-14.0
+-14.2
+-16.0
+-16.3
+-1c.0 | -+-3a {PCI bridge}
\-00.0 ** PREVIOUSLY NOT DETECTED!
+-1c.2 | -+-3c {PCI bridge}
\-00.0 ** PREVIOUSLY NOT DETECTED!
+-1d.0 {PCI bridge - no devices here}
+-1f.0
+-1f.2
+-1f.3
+-1f.4
\-1f.6
Signed-off-by: Francesco Romani <[email protected]>
ffromani
force-pushed
the
snapshot-pci-tree
branch
from
341af48 to
1f10653
Compare
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 current implementation of the PCI device snapshot only scans the first level in the PCI tree (actually, PCI forest). Anything attached to bridges is ignored. This was unintended, and this PR aims to fix it acknowledging bridges explictely and handling them correctly. In practices, this means to scan also the devices attached to each bridge, recursively.
Note this affects only the snapshotting code.