linuxkpi: Add definition of THIS_MODULE

[amshafer]

This adds a definition of THIS_MODULE during LKPI_DRIVER_MODULE. We need to do this to have a global to point THIS_MODULE at, which helps DRM drivers check if they have the same owner. The base linuxkpi code ensures that the contents of THIS_MODULE are never read, so we point it at KBUILD_MODNAME for ease of debugging. There are a couple special cases where I had to manually create the global to point THIS_MODULE at.

This solves panics such as: https://github.com/amshafer/nvidia-driver/issues/21

See: https://reviews.freebsd.org/D44306 This should not be merged until the base review lands so we know what __FreeBSD_version to filter against.

[bzfbd]

Doesn't that mean you may have multiple modules with clashing "char *THIS_MODULE_VAL" global variables. Does our kernel linker do such a good job to contain them per-module so you do not have a symbol clash if you load two modules?

[amshafer]

THIS_MODULE_VAL gets turned into something like __this_module_drm_ in the case of drm.ko. Since the expanded name is derived from LINUXKPI_PARAM_PREFIX which already has to be unique to avoid sysctl naming collisions, we don't have to worry about THIS_MODULE_VAL clashes. Even if there was an error and something with a colliding name was added the linker should see two __this_module_drm_ variables and fail to load the kernel module. I haven't tested that last part but afaik that's how it should work?

Based on that I think every module should have a different name that gets used to populate these globals, so no duplicates variables or collisions should be going on.

[bzfbd]

You have C code which in multiple files has the same global variable defined:

char *THIS_MODULE_VAL;

Doesn't matter what that variable gets assigned.

[amshafer]

So in those C file parts of the change (ttm and drm_os_freebsd) THIS_MODULE_VAL is a macro that gets expanded, although you are right that due to the way it's written it does look like it's a regular global variable name. Since that's pretty ugly I've replaced it with LKPI_DECLARE_THIS_MODULE(KBUILD_MODNAME); to make it obvious that we are declaring a dynamically named global variable.

This doesn't change any of the names of the generated global variables though, just makes it more obvious what is happening. Here's what all of the generated globals are:

ashafer@mick:git/drm-kmod % for f in **/*.ko; do echo "$f:" ; nm $f | grep -i this_module ; done
amd/amdgpu/amdgpu.ko:
0000000000004bd0 d __this_module_amdgpu_
dmabuf/dmabuf.ko:
drm/drm.ko:
0000000000000e70 d __this_module_drm_
i915/i915kms.ko:
0000000000000000 d __this_module_i915_
linuxkpi/linuxkpi_gplv2.ko:
linuxkpi_video/linuxkpi_video.ko:
radeon/radeonkms.ko:
00000000000073b0 d __this_module_radeon_
ttm/ttm.ko:
0000000000000070 d __this_module_ttm_

[wulf7]

maybe we can do something like: #define THIS_MODULE ((void *)string_hash(KBUILD_MODNAME))

There are couple of examples of build-time string hashing functions floating around like: https://stackoverflow.com/questions/28654707/how-to-calculate-the-hash-of-a-string-literal-using-only-the-c-preprocessor

Any thoughts?

[wulf7]

THIS_MODULE was added to LinuxKPI in 28a59100b54.

Closing this.