prefer dkms ${kernelver} over uname -r resolves #791

[josephtingiris]

No description provided.

[CLAassistant]

All committers have signed the CLA.

[aritger]

Thanks for the proposed change, @josephtingiris .

But, isn't mapping from dkms' ${kernelver} variable to KERNEL_UNAME a more suitable job for dkms.conf? This Makefile is generally not dkms-aware and already provides KERNEL_UNAME as the API to specify the target kernel.

In fact, it looks like dkms.conf already does KERNEL_UNAME=${kernelver}:
https://github.com/NVIDIA/open-gpu-kernel-modules/blob/main/kernel-open/dkms.conf#L9

Is that not working for some reason in your configuration?

[josephtingiris]

Thanks for the proposed change, @josephtingiris .

But, isn't mapping from dkms' ${kernelver} variable to KERNEL_UNAME a more suitable job for dkms.conf? This Makefile is generally not dkms-aware and already provides KERNEL_UNAME as the API to specify the target kernel.

In fact, it looks like dkms.conf already does KERNEL_UNAME=${kernelver}: https://github.com/NVIDIA/open-gpu-kernel-modules/blob/main/kernel-open/dkms.conf#L9

Is that not working for some reason in your configuration?

No, that dkms.conf doesn't work and I noted why, here: NVIDIA/yum-packaging-nvidia-driver#10 (comment)

IMO, the Makefile change is a way to keep the logic & functionality contained & maintained in a single location. There are already multiple dkms.conf files in play (also noted in the link above). Keep it simple.

The logic I added will preserve backward compatibility. $KERNEL_UNAME from the environment will still be honored and take precedence unless $kernelver is also present. If both are present then it tests for equality and will error if they don't match (which is how I found the issue to begin with). That seems more apropos. I thought it was better/safer behavior for make to error than to compile a module that fails to load on the next reboot (and renders the system significantly less useful without its gui). In my testing, the modules compile just fine but the version magic doesn't match. That's why it won't load.

[aritger]

I'm sorry, but the Makefile here isn't dkms-aware, and I don't believe it should be dkms-aware. If there are problems with dkms.conf, let's please fix that.

[josephtingiris]

No need. Happy to help if I can. I previously outlined fixing dkms.conf in the packaging repo, too. Those repos are quite dated.

The kernel-open/dkms.conf that's in this repo looks like a template for something else. It's also 3 years old. By itself, it doesn't function. Though it could easily be made to. I see other bits of dkms in here, too, though not in the Makefile. Are you implying the dkms.conf here get fixed?

Also, I searched and reviewed all of https://github.com/search?q=org%3ANVIDIA%20dkms.conf&type=code and the dkms.conf that's included in the rpm doesn't match any of them. I also searched for matching content and can't find it. I'll keep looking but I'm not sure how one would effect a change to that particular dkms.conf. I guess it's in a private repo.

Thanks for your time.

[1Naim]

The kernel-open/dkms.conf that's in this repo looks like a template for something else. It's also 3 years old

Yes you're right. It's a template from the NVIDIA runfile. The runfile will update dkms.conf as needed as per nvidia-installer/files.c but that's obviously not an option for packagers and I assume this repo too.

At least in Arch, they're making the modifications during the packaging stage so dkms.conf is useable. See [1] and [2]

[1] dkms.conf patching for the closed kernel modules
[2] dkms.conf patching for the open kernel moduels -- Also dkms.conf is moved to the top directory because otherwise it won't build due to missing targets.