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

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

[Dave Jiang]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]


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.

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

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

[Dave Jiang]

[-- Attachment #1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2: attachment.htm --]
[-- Type: text/html, Size: 3921 bytes --]

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

[Luck, Tony]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

       -r, --remove

           This option causes modprobe to remove rather than insert a

module. If the modules it depends on are also unused, modprobe will try to remove them

too. Unlike insertion, more than one module can be specified on the command line (it

does not make sense to specify module parameters when removing modules).



Maybe a bug in modprobe?  It shouldn’t try to remove idxd when the first of the

dependent modules is removed (because idxd is still “in use” by the other).



Either that, or IDXD and these two dependent modules aren’t setting the right

bits to let modprobe know what is going on?



-Tony



[-- Attachment #2: attachment.htm --]
[-- Type: text/html, Size: 3265 bytes --]

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

[Dave Jiang]

[-- Attachment #1: Type: text/plain, Size: 3455 bytes --]


On 4/6/2021 11:43 AM, Thomas, Ramesh wrote:
> On Tue, Apr 06, 2021 at 10:44:30AM -0700, Dave Jiang wrote:
>> 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.
> It returns an error and prints "FATAL" in the message so it does look
> like a real bug. I think I know what is going on, though I don't know
> why. When the second module is unloaded, modprobe also unloads idxd.
> Which means whenever it unloads either idxd_mdev or idxd_uacce, it tries
> to unload idxd and fails if another module dependent on idxd is still
> loaded blocking idxd from getting unloaded. Do you think idxd should
> maintain a reference count or similar indication so modprobe does not
> try to unload it when a dependent module is unloaded while other
> dependent ones are still active?

Ah I guess that makes sense. But idxd actually knows nothing about the 
other drivers depending on it. So I don't think we can do anything about 
that. I think that is the behavior of modprobe -r where it tries to 
unload all the relevant modules. I think for our purpose, rmmod works 
better as it just unloads the module you specify. Here's from the man 
page. Essentially it tries to remove the other module. But I guess it's 
not smart enough to be aware of dependency from some other module on the 
module it attempts to remove.


        -r, --remove
            This option causes modprobe to remove rather than insert a 
module. If the modules it
            depends on are also unused, modprobe will try to remove them 
too. Unlike insertion, more
            than one module can be specified on the command line (it 
does not make sense to specify
            module parameters when removing modules).


>
>>
>>> 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.
> rmmod does not complain. For now I can us it instead of modprobe.
>
>

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

[Thomas, Ramesh]

[-- Attachment #1: Type: text/plain, Size: 2418 bytes --]

On Tue, Apr 06, 2021 at 10:44:30AM -0700, Dave Jiang wrote:
> 
> 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.

It returns an error and prints "FATAL" in the message so it does look
like a real bug. I think I know what is going on, though I don't know
why. When the second module is unloaded, modprobe also unloads idxd.
Which means whenever it unloads either idxd_mdev or idxd_uacce, it tries
to unload idxd and fails if another module dependent on idxd is still
loaded blocking idxd from getting unloaded. Do you think idxd should
maintain a reference count or similar indication so modprobe does not
try to unload it when a dependent module is unloaded while other
dependent ones are still active?

> 
> 
> >
> > 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.

rmmod does not complain. For now I can us it instead of modprobe.

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

[Dave Jiang]

[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]


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

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

[Thomas, Ramesh]

[-- Attachment #1: Type: text/plain, Size: 2033 bytes --]

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.

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. 


> 
> 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

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

[Dave Jiang]

[-- Attachment #1: Type: text/plain, Size: 3013 bytes --]


On 4/5/2021 11:11 PM, Thomas, Ramesh wrote:
> 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.

That is definitely strange. I wonder if it has to do with 
idxd/sysfs/udev. Guess not a big deal if we need to wait for things to 
settle.

>
> 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.

Well for dsa_test, the idea was that we are running on a fresh slate and 
not inherit something from a previous test. That way any issue we found 
is no dependency on previous tests. Also after the test is run, we 
probably want to unload for another test or even just operation for 
something else. It also excercises the load/unload and see if we hit any 
issues from that.


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

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

[Thomas, Ramesh]

[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]

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

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

[Dave Jiang]

[-- Attachment #1: Type: text/plain, Size: 1832 bytes --]


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.


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

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

[Thomas, Ramesh]

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

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.

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

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

[Dave Jiang]

[-- Attachment #1: Type: text/plain, Size: 1254 bytes --]


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....


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