Repository navigation
Conversation
NewMigProfile groups every graphics variant with its plain counterpart except at three slices, so GPU_INSTANCE_PROFILE_3_SLICE_GFX falls through to the default arm and errors. The AttributeGraphics switch had the same hole, which would have cost the profile its +gfx suffix. The ID arrived with the go-nvml 0.13.2-0 bump, which also raised GPU_INSTANCE_PROFILE_COUNT from 17 to 18. That put the unhandled ID inside VisitMigProfiles' range, so on a driver reporting it as supported the error takes GetMigProfiles, ParseMigProfile and MigDevice.GetProfile down with it. The test mock groups the GFX variants with their plain counterparts, so it gets the 3-slice one too; without that the profile is reported unsupported and the path is never reached. Fixes NVIDIA#125 Signed-off-by: Dmitry Voropaev <dy.voropaev@gmail.com>
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
1 participant
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.
Fixes #125.
Two lines in the two
giProfileIDswitches inmig_profile.go: the 3-slice arm gainsGPU_INSTANCE_PROFILE_3_SLICE_GFX, and so does theAttributeGraphicsarm, so the profile stringifies as3g.<N>gb+gfxlike its 1-, 2- and 4-slice siblings instead of losing the suffix.The test mock gains the same grouping. That matters for coverage rather than style:
GetGpuInstanceProfileInfoFuncends indefault: return info, nvml.ERROR_NOT_SUPPORTED, so without it profile 17 is reported unsupported,VisitMigProfilesskips it, and the path is never exercised. Grouping it withGPU_INSTANCE_PROFILE_3_SLICEis what the mock already does for the other three graphics variants.TestNewMigProfileAcceptsEveryGraphicsSliceWidthwalks all four widths through the exportedNewMigProfile. Ond3f4ca8the 3-slice case fails withinvalid GPU Instance Profile ID: 17while the other three pass; here all four pass.The knock-on effect, measured with the extended mock, which is what a driver that exposes profile 17 looks like. Before:
After:
A plain
1g.5gblookup failing because of an unrelated profile ID is the part worth noting:VisitMigProfilesreturns hard on the first IDNewMigProfiledoes not know, so the gap is not contained to the graphics profile.go test -mod=vendor -count=1 ./...is green on all seven packages,gofmt -l pkg/andgo vet ./...clean, go 1.27.1 darwin/arm64. No vendor or go.mod change, somake check-vendorandmake check-third-party-noticesare unaffected. I have no hardware with the 3-slice graphics profile, so everything above is the repo's own mock rather than a real GPU.