level-zero icon indicating copy to clipboard operation
level-zero copied to clipboard

(on linux) the hardcoded driver names include the library version

Open fwyzard opened this issue 5 years ago • 9 comments

On Linux, the hardcoded library names include the major and minor library version, i.e.

MAKE_LIBRARY_NAME( "ze_intel_gpu", "0.4")

results in libze_intel_gpu.so.0.4.

Being hardcoded, this breaks loading the driver when its major or minor version changes (currently it is libze_intel_gpu.so.0.8).

Until a proper mechanism for enumerating the drivers is introduced, would it make sense to drop the version number from the library name also on Linux, as it is already the case on Windows ?

fwyzard avatar Mar 17 '20 10:03 fwyzard

See https://github.com/intel/compute-runtime/issues/279.

fwyzard avatar Mar 17 '20 10:03 fwyzard

Here is a patch:

diff --git a/source/inc/ze_util.h b/source/inc/ze_util.h
index 07a50c8..6aec3c8 100644
--- a/source/inc/ze_util.h
+++ b/source/inc/ze_util.h
@@ -21,6 +21,7 @@
 #  include <dlfcn.h>
 #  define HMODULE void*
 #  define MAKE_LIBRARY_NAME(NAME, VERSION)    "lib" NAME ".so." VERSION
+#  define MAKE_LIBRARY_NAMELINK(NAME)         "lib" NAME ".so"
 #  define MAKE_VALIDATION_LAYER_NAME(NAME)    "lib" NAME ".so." L0_VALIDATION_LAYER_SUPPORTED_VERSION
 #  define LOAD_DRIVER_LIBRARY(NAME) dlopen(NAME, RTLD_LAZY|RTLD_LOCAL)
 #  define FREE_DRIVER_LIBRARY(LIB)  if(LIB) dlclose(LIB)
@@ -28,6 +29,7 @@
 #elif defined(_WIN32)
 #  include <Windows.h>
 #  define MAKE_LIBRARY_NAME(NAME, VERSION)    NAME".dll"
+#  define MAKE_LIBRARY_NAMELINK(NAME)         NAME".dll"
 #  define MAKE_VALIDATION_LAYER_NAME(NAME)    NAME".dll"
 #  define LOAD_DRIVER_LIBRARY(NAME) LoadLibrary(NAME)
 #  define FREE_DRIVER_LIBRARY(LIB)  if(LIB) FreeLibrary(LIB)
diff --git a/source/loader/ze_loader.cpp b/source/loader/ze_loader.cpp
index 82e31df..6e5398a 100644
--- a/source/loader/ze_loader.cpp
+++ b/source/loader/ze_loader.cpp
@@ -11,7 +11,7 @@ namespace loader
 {
     ///////////////////////////////////////////////////////////////////////////////
     static const char* known_driver_names[] = {
-        MAKE_LIBRARY_NAME( "ze_intel_gpu", "0.4"),
+        MAKE_LIBRARY_NAMELINK( "ze_intel_gpu" ),
     };
 
     static const size_t num_known_driver_names =

Let me know if this makes sense, and if you would like a pull request.

fwyzard avatar Mar 17 '20 12:03 fwyzard

The loader's patches are behind driver's, but you should see soon the loader updated to work with level zero driver v0.8. Removing version can cause confusion while APIs are added or modified. We will improve our readme for pulling the right commit while building from source.

Thanks Ravi

ravindra-ganapathi avatar Mar 17 '20 15:03 ravindra-ganapathi

On Linux, the hardcoded library names include the major and minor library version, i.e.

MAKE_LIBRARY_NAME( "ze_intel_gpu", "0.4")

results in libze_intel_gpu.so.0.4.

Being hardcoded, this breaks loading the driver when its major or minor version changes (currently it is libze_intel_gpu.so.0.8).

Until a proper mechanism for enumerating the drivers is introduced, would it make sense to drop the version number from the library name also on Linux, as it is already the case on Windows ?

Both normally linked libraries and libraries that dlopen() should only use SONAME to ensure SEMVER style API/ABI compatibilty.

The SONAME currently exported from libze_intel_gpu.so contains both MAJOR and MINOR. This is a more precise variation on the SEMVER standard of only MAJOR. Since the ABI between the loader and driver has to be precisely matched, it is necessary to retain for the near term since the loader only supports a single version for the moment. Do please note this is vendor specific, should another vendor add a driver, they have their own control over versioning between loader and driver.

Additional background, today since the Level Zero specification is not at v1.0 and is pre-release, our initial implementation for SONAME is including the MINOR to avoid runtime issues that are fluxing to quickly. After v1.0 is released the SONAME will be switched for the intel-gpu driver to only using MAJOR and thus normal SEMVER will resume, hopefully very soon. So these breakages will become far less likely. Also community adoption of perhaps beyond simple SEMVER compatibility perhaps will be a thing.

If the community wants to implement backwards compatibility past normal SEMVER standards, that can be worked on, but will be complicated pending on how the Level Zero specification decides to handle their headers, functions, and structure names.

rwmcguir avatar Mar 17 '20 17:03 rwmcguir

The SONAME currently exported from libze_intel_gpu.so contains both MAJOR and MINOR. This is a more precise variation on the SEMVER standard of only MAJOR. Since the ABI between the loader and driver has to be precisely matched, it is necessary to retain for the near term since the loader only supports a single version for the moment. Do please note this is vendor specific, should another vendor add a driver, they have their own control over versioning between loader and driver.

So, if I understand correctly, once v1.0 is define, a loader version v1.x should be compatible with a library version v1.y: the major version (v1) will identify the ABI compatibility, and the minor version (x vs y) will identify differences in features that do not impede such (backward? ) compatibility.

If the community wants to implement backwards compatibility past normal SEMVER standards, that can be worked on, but will be complicated pending on how the Level Zero specification decides to handle their headers, functions, and structure names.

These seem more like API changes, that would make incompatible use fail at build time rather than at run time ?

fwyzard avatar Mar 17 '20 21:03 fwyzard

So, if I understand correctly, once v1.0 is define, a loader version v1.x should be compatible with a library version v1.y: the major version (v1) will identify the ABI compatibility, and the minor version (x vs y) will identify differences in features that do not impede such (backward? ) compatibility.

Correct, this is standard SEMVER: https://semver.org/
Which is the intention. v1.x+1 will work with v1.x support drivers, with perhaps a warning message if it can't find the symbol in the driver. Loader v1.x simply won't see new functions implemented by a v1.x+1 runtime library, but should still work fine.

These seem more like API changes, that would make incompatible use fail at build time rather than at run time ? The difference I was pointing out is if the loader was asked to be compatible between v1.x and v2.x, specification then structures or similar API's may change in small or major ways with the same name. How does one #include both versions of the Level Zero Specification and not break compilation? This gets tricky, and while clever solutions are possible it may not be worth it. Opening a file with extension .so at runtime is non-deterministic for versions, so runtime may be forced start doing ABI/API checking per symbol. Not opening unversioned .so files and using SONAME alleviates the need for some or most of this. Attempting to avoid where application has to tell loader which major interface it wants and thus the logic in the loader to archive previous versions of all symbols.

While some parts of this will indeed need implemented just sticking with standard SEMVER will solve a lot of complexity and prevent bugs down the road. :-) But hey, this is community project, that is just my two cents.

rwmcguir avatar Mar 18 '20 00:03 rwmcguir

I see, thanks for sharing your point of view.

fwyzard avatar Mar 18 '20 05:03 fwyzard

Just FYI, the version has been updated with latest commit. Things should align with the binaries in the libze_intel_gpu.so.0.8 now.

rwmcguir avatar Mar 20 '20 23:03 rwmcguir

Thanks for the update.

fwyzard avatar Mar 20 '20 23:03 fwyzard