Repository navigation
Conversation
addCacheInfo does not report an L2 cache that several cores share. It keeps a level 1 or 2 cache only if it belongs to one core, so on CPUs where a few cores share an L2 (Xeon 6 with E-cores, the E-cores of Core i7-12700K) the cache is missing in machine info, although sysfs shows it. Handle such an L2 like the caches above L2: put it into Node.Caches if all CPUs of the node share it, otherwise into Core.UncoreCaches. A cache private to one core stays in Core.Caches. L3 handling is not changed. In UncoreCaches the L2 now comes before the L3, so code should use Level to find an entry instead of its position in machine info. The tests use a fake sysfs with different caches for each CPU: 2-core and 4-core L2 clusters, an L2 shared by the whole node, SMT cores with a private L2, and the layout of Core i7-12700K. ompared machine info from several hosts before and after the change, and it differs only where an L2 is shared by several cores (Xeon with E-cores, hybrid CPUs)
addCacheInfo reports a cache in Node.Caches when the CPUs that share it are all the threads of the node. It multiplies the number of cores by the threads of the current core to get the threads of the node. This is wrong when the cores have different numbers of threads. Hybrid CPUs are an example. Core i7-12700K has 8 cores with SMT and 4 cores without SMT. The node has 20 threads. The function computes 24 for a core with SMT and 12 for the other cores. The L3 cache is shared by all 20 threads, so it never matches. cadvisor lists it in Core.UncoreCaches of every core and not in Node.Caches. Add the threads of all cores of the node instead. The L3 cache of the Core i7-12700K is now in Node.Caches. A node where all cores have the same number of threads gives the same result as before. The metric machine_cpu_cache_capacity_bytes shows this cache once for the node, not once for each core. The new test uses the layout of Core i7-12700K. It fails without this change. It builds on the fake sysfs of the change that reports L2 caches shared by several cores.
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.
addCacheInfo reports a cache in Node.Caches when the CPUs that share it
are all the threads of the node. It multiplies the number of cores by
the threads of the current core to get the threads of the node. This is
wrong when the cores have different numbers of threads. Hybrid CPUs are
an example.
Core i7-12700K has 8 cores with SMT and 4 cores without SMT. The node
has 20 threads. The function computes 24 for a core with SMT and 12 for
the other cores. The L3 cache is shared by all 20 threads, so it never
matches. cadvisor lists it in Core.UncoreCaches of every core and not in
Node.Caches.
Add the threads of all cores of the node instead. The L3 cache of the
Core i7-12700K is now in Node.Caches. A node where all cores have the
same number of threads gives the same result as before. The metric
machine_cpu_cache_capacity_bytes shows this cache once for the node, not
once for each core.
The new test uses the layout of Core i7-12700K. It fails without this
change. It builds on the fake sysfs of the change that reports L2 caches
shared by several cores.
Depends on: #3954