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

cpuallocator: don't filter based on single CoreKind. #345

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

klihub
Copy link
Collaborator

@klihub klihub commented Jun 26, 2024

Only do CoreKind filtering in clustered allocation when we have multiple CoreKinds available, IOW on hybrid core architectures.

Copy link
Collaborator

@fmuyassarov fmuyassarov left a comment

Choose a reason for hiding this comment

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

This looks good based on what title & description says. But would you mind to share why is this needed?

@klihub
Copy link
Collaborator Author

klihub commented Jun 26, 2024

This looks good based on what title & description says. But would you mind to share why is this needed?

Yes. Sorry, I should have included an explanation in the commit message as well and will do so now.
So without this relaxed filtering, if we have only one core type present (for instance E-cores) and the caller indicates a priority preference which corresponds to an unavailable core type (for instance high-prio/P-core), we won't do clustered allocation at all, because the initial filtering based on core type will filter out all available clusters as unsuitable. But acting so (and for a preference) does not make sense when we only have one type of cores, hence this change.

If we have only one type of core present (for instance E-core)
and the caller indicates a priority preference which corresponds
to an unavailable core type (for instance high-prio/P-core), we
don't currently attempt clustered allocation at all, since the
initial CoreKind based filtering filters out all CPU clusters as
unsuitable.

This does not make sense so relax this and only do core type
filtering when we have multiple of them (when running on hybrid
core architectures).

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

@fmuyassarov fmuyassarov 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 .

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 2674a29 into containers:main Jul 1, 2024
2 checks passed
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.

3 participants