Skip to content
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

sysfs,cpuallocator: add support for hybrid core discovery, preferred allocation. #295

Merged
merged 6 commits into from
Mar 27, 2024

Conversation

klihub
Copy link
Collaborator

@klihub klihub commented Mar 18, 2024

Notes: This PR is stacked on top of #294 and #206.

This patch series adds support for discovering CPU cores of various kinds on hybrid core architecture systems. It also updates the CPU allocator to make better allocation choices on such systems, both with and without explicitly provided allocation preferences.

@klihub klihub force-pushed the devel/hybrid-core-support branch 2 times, most recently from b541412 to f2065e1 Compare March 18, 2024 15:48
@fmuyassarov
Copy link
Collaborator

@klihub can you please rebase this one ?

Add support for hybrid core architectures by introducing core types.
Add an interface for querying the available core types. Define core
types for P- and E-cores and implement detection for those types.

Signed-off-by: Krisztian Litkey <[email protected]>
@klihub
Copy link
Collaborator Author

klihub commented Mar 21, 2024

@klihub can you please rebase this one ?

@fmuyassarov Rebased on latest main.

@klihub klihub force-pushed the devel/hybrid-core-support branch 2 times, most recently from 2f0800b to 45cfa38 Compare March 23, 2024 18:02
Copy link
Collaborator

@marquiz marquiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @klihub. Looks pretty good

One nit about one of the commit messages: s/sorring/sorting/

pkg/sysfs/system.go Outdated Show resolved Hide resolved
pkg/cpuallocator/allocator.go Outdated Show resolved Hide resolved
pkg/cpuallocator/allocator.go Outdated Show resolved Hide resolved
@klihub
Copy link
Collaborator Author

klihub commented Mar 25, 2024

Thanks @klihub. Looks pretty good

One nit about one of the commit messages: s/sorring/sorting/

Thanks for spotting it. Fixed.

Allow overriding automatic core type detection using environment
variables. Among other things, this can be used for e2e tests to
emulate a hybrid architecture on non-hybrid hosts.

Signed-off-by: Krisztian Litkey <[email protected]>
Update sample/test sysfs data to report multiple/hybrid core types.

Signed-off-by: Krisztian Litkey <[email protected]>
@klihub klihub requested a review from marquiz March 26, 2024 12:07
Copy link
Collaborator

@askervin askervin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to double-check CPU set comparison

pkg/cpuallocator/allocator.go Outdated Show resolved Hide resolved
pkg/cpuallocator/allocator.go Outdated Show resolved Hide resolved
pkg/cpuallocator/allocator.go Outdated Show resolved Hide resolved
pkg/cpuallocator/allocator.go Outdated Show resolved Hide resolved
pkg/cpuallocator/allocator.go Outdated Show resolved Hide resolved
Classify all cores reported as E-cores by sysfs as low
priority cores without looking at their cpufreq range.
When sorting CPU sets for low-prio allocations, prefer
the tightest fitting set which can satisfy the request
with low-prio CPUs.

Signed-off-by: Krisztian Litkey <[email protected]>
When sorting CPU sets for high-prio allocations, prefer
the tightest fitting set which can satisfy the request
with high-prio or normal-prio CPUs.

Signed-off-by: Krisztian Litkey <[email protected]>
When allocating idle packages, consider a package idle if all
online cores *with preferred priorities* for that allocation
are idle. Also, from the picked packages take cores with this
same restriction. In particular, on hybrid architectures with
multiple core types, this will exclude
  - E-cores from allocations with Priority{Normal,High} preference
  - P-cores from allocations with PriorityLow preference

Signed-off-by: Krisztian Litkey <[email protected]>
Copy link
Collaborator

@marquiz marquiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @klihub for the tireless work on this one. Now even I can understand how it works 😸 At the same time wondering when we reach the point that nobody understands how the cpuallocator actually works...

LGTM

@marquiz
Copy link
Collaborator

marquiz commented Mar 26, 2024

@askervin PTAL. When this is merged let's rebase #282 and dive into it

Copy link
Collaborator

@askervin askervin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@askervin askervin merged commit d8d3407 into containers:main Mar 27, 2024
2 checks passed
@klihub klihub deleted the devel/hybrid-core-support branch March 27, 2024 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants