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

Change GPU plugin's behavior as Level zero's default hierarchy mode changed from composite to flat #1601

Merged
merged 4 commits into from
Dec 8, 2023

Conversation

tkatila
Copy link
Contributor

@tkatila tkatila commented Nov 20, 2023

Use "FLAT" affinity mask mode by default and do not set mask when there are only 1-tile GPUs.

Re-structure GPU's README:
-Simplify the main readme, move "nice to know" info to separate pages.
-Highlight only one installation method, rest are available on different page.

@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (5b7ec1f) 51.79% compared to head (033bd20) 51.95%.

❗ Current head 033bd20 differs from pull request most recent head 95b7230. Consider uploading reports for the commit 95b7230 to get more accurate results

Files Patch % Lines
cmd/gpu_plugin/rm/gpu_plugin_resource_manager.go 89.36% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1601      +/-   ##
==========================================
+ Coverage   51.79%   51.95%   +0.15%     
==========================================
  Files          42       42              
  Lines        4873     4916      +43     
==========================================
+ Hits         2524     2554      +30     
- Misses       2204     2213       +9     
- Partials      145      149       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@eero-t eero-t left a comment

Choose a reason for hiding this comment

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

On quick look code / tests looked OK, but I have some comments, mostly for updating docs a bit.

cmd/gpu_plugin/README.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/README.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/README.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/README.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/README.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/driver-firmware.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/driver-firmware.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/fractional.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/fractional.md Outdated Show resolved Hide resolved
cmd/gpu_plugin/gpu_plugin.go Outdated Show resolved Hide resolved
@tkatila
Copy link
Contributor Author

tkatila commented Nov 21, 2023

I added a picture for the "operation modes" to explain the three different modes.

Copy link
Contributor

@eero-t eero-t left a comment

Choose a reason for hiding this comment

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

Looks good, but I still have few improvement suggestions.

cmd/gpu_plugin/gpu_plugin.go Outdated Show resolved Hide resolved
cmd/gpu_plugin/README.md Show resolved Hide resolved
cmd/gpu_plugin/usage-scenarios.png Outdated Show resolved Hide resolved
eero-t
eero-t previously approved these changes Nov 21, 2023
Copy link
Contributor

@eero-t eero-t left a comment

Choose a reason for hiding this comment

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

/LGTM Everything looks good to me now!

eero-t
eero-t previously approved these changes Nov 23, 2023
tkatila and others added 4 commits December 8, 2023 08:40
With tile requests, the level zero affinity mask now defaults to
flat/combined mode. If ZE_FLAT_DEVICE_HIERARCHY is set to COMPOSITE
in the Pod's specification, plugin will use the previous "x.y" format
instead of the new "x" in the affinity mask.

Signed-off-by: Tuomas Katila <[email protected]>
Split readme into smaller chunks, show only one "easy installation"
and hide the rest. Add some notes about tile resources.

Co-authored-by: Eero Tamminen <[email protected]>
Signed-off-by: Tuomas Katila <[email protected]>
@tkatila
Copy link
Contributor Author

tkatila commented Dec 8, 2023

Rebased. No functional changes.

@mythi mythi merged commit c5c1127 into intel:main Dec 8, 2023
77 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.

5 participants