intel / idxd-config

Accel-config / libaccel-config
Other
61 stars 35 forks source link

4.1.6 dsa_user_test_runner fails #60

Closed snits closed 4 months ago

snits commented 8 months ago

With the following commit:

29cdf35f38da ("accel-config/test: disable enabled devices before unloading iaa_crypto")

dsa_user_test_runner.sh fails on a system that doesn't have iax devices:

+ for device_type in 'dsa' 'iax'
+ '[' -d /sys/bus/iax ']'
+ DSA_DEVICE_PATH=/sys/bus/dsa/devices
+ for device_path in ${DSA_DEVICE_PATH}/${device_type}*
++ echo '/sys/bus/dsa/devices/iax*'
++ grep -c '!'
+ [[ 0 -eq 0 ]]
+ for wqp in ${device_path}/wq*
++ cat '/sys/bus/dsa/devices/iax*/wq*/state'
cat: '/sys/bus/dsa/devices/iax*/wq*/state': No such file or directory
+ [[ '' == \e\n\a\b\l\e\d ]]
++ cat '/sys/bus/dsa/devices/iax*/state'
cat: '/sys/bus/dsa/devices/iax*/state': No such file or directory
+ [[ '' == \e\n\a\b\l\e\d ]]
+ for engine in ${device_path}/engine*
+ echo -1
./common: line 148: /sys/bus/dsa/devices/iax*/engine*/group_id: No such file or directory
++ err 148
+++ basename ./dsa_user_test_runner.sh
++ echo test/dsa_user_test_runner.sh: failed at line 148
test/dsa_user_test_runner.sh: failed at line 148
++ '[' -n '' ']'
++ exit 77
snits commented 8 months ago

Is the code that checks for iax_crypto in _cleanup() just left over from before it was called iaa_crypto and merged into the kernel?

snits commented 8 months ago

Also is there anything that ever actually used /sys/bus/iax ? To configure the workqueues and devices you would've needed the workqueue driver which was merged long after the iax bus_type disappeared, yes?

ramesh-thomas commented 7 months ago

Also is there anything that ever actually used /sys/bus/iax ? To configure the workqueues and devices you would've needed the workqueue driver which was merged long after the iax bus_type disappeared, yes?

"/sys/bus/iax" is not a valid path. Both iax and dsa devices are under /sys/bus/dsa. @fyu1 has this changed?

@ysun can you please fix this?

fyu1 commented 7 months ago

I'm not aware of /sys/bus/iax path. As Ramesh said, all iax and dsa devices are in /sys/bus/dsa dir.

ramesh-thomas commented 7 months ago

I actually forgot that there was an issue when the ABI was getting changed. Based on accel-config code, I think some version of the idxd driver put iax devices under /sys/bus/iax so we added code in accel-config to check that path also.

@davejiang do we still need to support the /sys/bus/iax path?

snits commented 7 months ago

It used to exist in the kernel, but it was removed before the workqueue driver existed:

1c264299431e dmaengine: idxd: remove iax_bus_type prototype | 2021-07-21 | (Dave Jiang)

I just wasn't sure if that check in the script was really needed. Going back and looking, there was some code for the iax devices. I don't recall if we had any systems with iax back then:

f25b463883a8 dmaengine: idxd: add IAX configuration support in the IDXD driver | 2020-12-11 | (Dave Jiang)

ysun commented 7 months ago

Missed message. Get it. From the Tony's commit log, that check is for kernel v5.13 which we never use now. I'm going to fix it.

ramesh-thomas commented 7 months ago

@snits can you please confirm if Redhat needs to support kernel v5.13? If it does then we cannot remove the check for /sys/bus/iax path.

If @ysun removes /sys/bus/iax path then will we still need your patch https://github.com/intel/idxd-config/pull/62/commits/9b1433a19192c129f37863f9ec24e79206d3aa10 ?

ramesh-thomas commented 7 months ago

@snits I spoke with @davejiang and he mentioned that we can remove the check for /sys/bus/iax if it is ok with Redhat. The assumption is the kernel v5.13 idxd driver may not be used by other distributions with accel-config.

@ysun please don't remove the code for now until @snits confirms.

snits commented 7 months ago

I need to take a look at RHEL8.6, which would be the only release potentially impacted. I should have in the next couple of days.