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

On Mon, Apr 05, 2021 at 09:11:54PM -0700, Dave Jiang wrote:
> 
> On 4/5/2021 6:10 PM, Thomas, Ramesh wrote:
> > On Mon, Apr 05, 2021 at 05:53:21PM -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>
> >>> ---
> >>>    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
> >> What is the reason having to remove idxd_mdev twice? idxd_uacce
> >> shouldn't have any dependency on blocking idxd_mdev from loading....
> > The dependncy order of idxd_uacce and idxd_mdev is not fixed. The one
> > that has the other as a dependent would fail if unloaded first. This
> > workaround takes care of both cases, though it is not clean and would be
> > better if driver ensures a fixed dependency order.
> 
> Huh I'm surprised that there is dependency between the two at all. They
> each really should be dependent on idxd.

It complains about idxd being busy so it is possible the dependency is
inside idxd. However, if the unload order is switched between uacce and
mdev, then it would unload ok. The failing order is random.

Should we remove the code doing the unload and reload of modules in
dsa_test? I don't think we should need to unload and reload drivers
after every test which will only hide driver instability issues.

> 
> 
> >>
> >>> @@ -108,6 +116,7 @@ _cleanup()
> >>>    sleep 1
> >>>    }
> >>>    modprobe idxd
> >>> +sleep 1
> >>>    }
> >>>
> >>>    # json2var