search

intel / idxd-config

Accel-config / libaccel-config
Other
56 stars 34 forks source link

make kmod dependency optional #10

Closed mythi closed 2 years ago

mythi commented 2 years ago

We're planning to ship accel-config in a container and want to minimize the dependencies needed to be distributed. Since we can control that accel-config is only run on systems where idxdalready loaded, the kmod check is redundant. Can you make the kmoddependency a configure option build time?

ramesh-thomas commented 2 years ago

Do you mean the test checking for idxd module? The check is there so it would fail if anyone tries to run it without idxd is loaded and they know why it is failing. e.g. idxd module could have failed to load due to some error.

Are you facing any issue with "make check:" or installation failing? If yes, then we would need to disable the check that runs the test.

mythi commented 2 years ago

Are you facing any issue with "make check:" or installation failing?

No. My ask is to be able to say ./configure --disable-kmod-checks to get accel-config without the module check.

The check is there so it would fail if anyone tries to run it without idxd is loaded and they know why it is failing

In my environment I can check this separately so I can safely skip it with accel-config

ramesh-thomas commented 2 years ago

The kmod check is done only in the unit test. When do you see the test getting run without you explicitly running it? Can you give the commands you are running and the error you see when idxd kernel module is not loaded?

The test will be disabled if you don't include "--enable-test=yes" in the configure command. If you are generating rpms then use rpmbuild.sh instead of rpmbuild-test.sh

mythi commented 2 years ago 
# accel-config list
Failed initializing kernel module
ramesh-thomas commented 2 years ago

That is an actual command failing and not an installation issue due to kmod dependency. All accel-config operations including the list command require interaction with the idxd kernel module.

mythi commented 2 years ago

See my original ask. I have nothing failing. I can be sure the driver is loaded so I don’t need kmod to check it.

ramesh-thomas commented 2 years ago

in a container and want to minimize the dependencies needed @mythi I understand the redundancy of kmod loading idxd but not sure what dependency are you referring to here. Can you give more details so we can be aware of the issues faced in containers?

Following is my analysis about the redundancy argument: Currently accel-config uses kmod to load the driver if not loaded and increments the reference count. I think for accel-config usage which doesn't stay resident, the reference count may not be necessary. It can also check sysfs entry to verify the presence of the kernel module. I prefer removing it altogether as most users would have idxd kernel module loaded. We can provide an initialization script if necessary to load the driver. @aegl @davejiang let me know your opinion.

davejiang commented 2 years ago

@ramesh-thomas that is fine by me.

mythi commented 2 years ago

I prefer removing it altogether as most users would have idxd kernel module loaded.

Works for me!

mythi commented 2 years ago

is there a release planned with this change?

ramesh-thomas commented 2 years ago

Did not get a chance to do it yet. Will try to make the change after I return from vacation next week.

mythi commented 2 years ago

@ramesh-thomas OK, thanks. There seems to be a dependency to udev as well for no obvious reason. Could it be dropped too while we’re at it?

ramesh-thomas commented 2 years ago

I will take a look and if not required will remove it. Thanks.

ramesh-thomas commented 2 years ago

kmod and udev are removed in new release https://github.com/intel/idxd-config/releases/tag/accel-config-v3.4.3

mythi commented 2 years ago

@ramesh-thomas thanks! We'll move to that release ASAP