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

Enumerate Nvidia GPU IDs #1840

Merged
merged 3 commits into from
Apr 22, 2021
Merged

Enumerate Nvidia GPU IDs #1840

merged 3 commits into from
Apr 22, 2021

Conversation

jailuthra
Copy link
Contributor

@jailuthra jailuthra commented Apr 14, 2021

What does this pull request do? Explain your changes. (required)

Automatically utilize all available Nvidia GPUs for transcoding, if the user sets the command line parameter as -nvidia=all or -nvidia all.

Specific updates (required)

See commit history

How did you test each of these updates (required)

Manually tested, and verified correct behaviour on:

  • Linux desktop with a single GTX 1060 6gb
  • Linux intel laptop with no nvidia GPU
  • Berlin staging pod with multiple nvidia GPUs
  • Eli's test on a windows machine

Does this pull request close any open issues?

Fixes #1205

Checklist:

@jailuthra jailuthra changed the title Implement automatic gpu device id enumeration for Nvidia Enumerate Nvidia GPU IDs Apr 14, 2021
@iameli
Copy link
Member

iameli commented Apr 15, 2021

Tested on Windows, working great!

image

common/util.go Outdated Show resolved Hide resolved
common/util.go Outdated Show resolved Hide resolved
common/util.go Outdated Show resolved Hide resolved
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

Since we're using a package that gives us access to all device info now, an additional UX improvement could be to log the Nvidia device names that the node is configured with on startup so it is clear to the user what devices are being used. Feel free to consider this for a separate PR though.

common/util.go Outdated Show resolved Hide resolved
common/util.go Outdated Show resolved Hide resolved
common/util.go Outdated Show resolved Hide resolved
common/util.go Outdated Show resolved Hide resolved
@jailuthra
Copy link
Contributor Author

jailuthra commented Apr 17, 2021

Fixed pending comments, and squashed+rebased on top of latest master. Also did some sanity testing on my 1060 again.

an additional UX improvement could be to log the Nvidia device names that the node is configured with on startup so it is clear to the user what devices are being used. Feel free to consider this for a separate PR though.

That would be a very cool feature. Will open a separate tracking issue for it.

doc/gpu.md Show resolved Hide resolved
cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
cmd/livepeer_bench/livepeer_bench.go Outdated Show resolved Hide resolved
@kyriediculous
Copy link
Contributor

Requested changes look good to me, will let Yondon follow up on his change requests seperately.

Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

LGTM

Windows build is failing due to what looks like a flaky gpg issue. Please trigger another build just to check if the Windows build passes and then this should be good to merge.

@jailuthra
Copy link
Contributor Author

Squashed the fixup to trigger windows re-build. Everything works now. Merging.

@jailuthra jailuthra merged commit 7a0fd1f into master Apr 22, 2021
@yondonfu yondonfu deleted the jai/nvdevices branch April 22, 2021 13:52
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.

Implement -nvidia=all
4 participants