[Accel-config] Re: [PATCH] accel-config/test: load/unload uacce module at cleanup

On 4/6/2021 10:32 AM, Thomas, Ramesh wrote:
> On Tue, Apr 06, 2021 at 08:55:43AM -0700, Dave Jiang wrote:
>> On 4/5/2021 5:45 PM, ramesh.thomas(a)intel.com wrote:
>>> From: Ramesh Thomas <ramesh.thomas(a)intel.com>
>>>
>>> Cleanup unloads and reloads all idxd kernel modules. This is broken due
>>> to idxd_module depending on idxd module. The order of idxd_mdev and
>>> idxd_uacce modules are not fixed requiring additional checks during the
>>> process.
>>>
>>> Signed-off-by: Tony Zhu <tony.zhu(a)intel.com>
>>> Signed-off-by: Ramesh Thomas <ramesh.thomas(a)intel.com>
>> I would maybe add a comment explaining the weird behavior observed.
> I have a new finding. Even though "modprobe -r" returns failure and
> prints error message "modprobe: FATAL: Module idxd is in use.", the
> module is still unloaded. It does not give that error on the second
> module that is unloaded so I earlier thought it has to do with the
> order. This seems to be a bug in the driver and is easily reproducible.

Hmm.....if so it would be more in the device driver framework than the 
driver. The driver only supply the init/remove callbacks. It doesn't 
have any control over the load/unload mechanism. It can hold ref count 
on the module, but we don't do that.


>
> Since it returns failure, the test script aborts with error. I will
> modify the workaround to not trap on the error and continue. That way we
> don't need the second unload attempt of the same module. I will add
> a comment describing this as a workaround for the above behavior.
>
Does rmmod complain? That's usually what I use.


>> Reviewed-by: Dave Jiang <dave.jiang(a)intel.com>
>>
>>
>>> ---
>>>    test/common | 9 +++++++++
>>>    1 file changed, 9 insertions(+)
>>>
>>> diff --git a/test/common b/test/common
>>> index 4bd4ed3..e6e84dd 100644
>>> --- a/test/common
>>> +++ b/test/common
>>> @@ -99,6 +99,14 @@ check_prereq()
>>>    #
>>>    _cleanup()
>>>    {
>>> +lsmod | grep -q "idxd_mdev" && {
>>> +modprobe -r idxd_mdev 2>/dev/null || :
>>> +sleep 1
>>> +}
>>> +lsmod | grep -q "idxd_uacce" && {
>>> +modprobe -r idxd_uacce
>>> +sleep 1
>>> +}
>>>    lsmod | grep -q "idxd_mdev" && {
>>>    modprobe -r idxd_mdev
>>>    sleep 1
>>> @@ -108,6 +116,7 @@ _cleanup()
>>>    sleep 1
>>>    }
>>>    modprobe idxd
>>> +sleep 1
>>>    }
>>>
>>>    # json2var