grgalex / nvshare

Practical GPU Sharing Without Memory Size Constraints
Apache License 2.0
228 stars 24 forks source link

[fix] support cuGetProcAddress and cuGetProcAddress_v2 #13

Closed pokerfaceSad closed 11 months ago

pokerfaceSad commented 11 months ago
pokerfaceSad commented 11 months ago

@grgalex PTL, fix #11

grgalex commented 11 months ago

Thanks for the PR!

Some minor comments:

  1. It makes sense to split into 2 commits, the first only touching the existing cuGetProcAddress, where we add the initialize_libvnshare() call. The 2nd commit will add support for cuGetProcAddress_v2.
  2. We want concise commit titles, which start with the subsystem that the commit touches. In this case:
    1. hook: Add conditional library initialization in cuGetProcAddress()
    2. hook: Add hook for cuGetProcAddress_v2() to support CUDA >=12
  3. We also want descriptive commit messages, which first give some background and finally describe the actions taken to tackle the issue. In this case, you can take a look at your own comment [1] for inspiration :)
  4. Use the same email address as your GH account when committing. If you want to use a different address, you can add it to your GH account via the settings.
  5. Sign your commits (git commit -sm), so they have a Signed-off-by: line at the end. Use your name and email address. Look at the existing commits for a reference.
  6. Add a line with Closes #11 at the end of the commit message with one line empty before and one after, before the Signed-off-by: line. This will automatically close issue #11 once we merge.

Can you apply these changes and re-push? If you need any help, feel free to ask for it!

[1] https://github.com/grgalex/nvshare/issues/11#issuecomment-1835379345

pokerfaceSad commented 11 months ago

@grgalex submit another PR in #14