[Intel-gfx] [PATCH i-g-t] tests/drv_module_reload_basic: Don't use rmmod exit code when reloading the module.

[Chris Wilson]

On Tue, May 24, 2016 at 09:03:19AM +0100, Chris Wilson wrote:
> On Tue, May 24, 2016 at 09:55:26AM +0200, Daniel Vetter wrote:
> > On Mon, May 23, 2016 at 01:43:42PM +0300, Marius Vlad wrote:
> > > On Fri, May 20, 2016 at 05:23:56PM +0100, Chris Wilson wrote:
> > > > On Fri, May 20, 2016 at 07:00:18PM +0300, Imre Deak wrote:
> > > > > On pe, 2016-05-20 at 18:20 +0300, Marius Vlad wrote:
> > > > > > Either we return $IGT_EXIT_FAILURE or remove it entirely (like in
> > > > > > this
> > > > > > patch). If rmmod returns non-zero (i.e., Module: i915 is still in
> > > > > > use), reload
> > > > > > will bail with $IGT_EXIT_SKIP, making the check with lsmod useless.
> > > > > > Also use the return value in the fault-injection loop.
> > > > > > 
> > > > > > Signed-off-by: Marius Vlad <marius.c.vlad at intel.com>
> > > > > > ---
> > > > > >  tests/drv_module_reload_basic | 4 ++--
> > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/tests/drv_module_reload_basic
> > > > > > b/tests/drv_module_reload_basic
> > > > > > index 3bba796..3a8df33 100755
> > > > > > --- a/tests/drv_module_reload_basic
> > > > > > +++ b/tests/drv_module_reload_basic
> > > > > > @@ -30,7 +30,7 @@ function reload() {
> > > > > >  
> > > > > >  	#ignore errors in ips - gen5 only
> > > > > >  	rmmod intel_ips &> /dev/null
> > > > > > -	rmmod i915 || return $IGT_EXIT_SKIP
> > > > > > +	rmmod i915
> > > > > 
> > > > > Not sure what was the reason to bail out here, continuing seems like
> > > > > the correct thing to do.
> > > > 
> > > > If we can't unload, we can perform the modprobe testing. The system is
> > > > not in a state suitable for testing so skip or fail. If we are certain
> > > > that the rmmod failure is a bug, fail, if it merely something like the
> > > > system doesn't support module unloading, skip.
> > > I've seen this couple of times in the CI...and went mostly mostly
> > > unnoticed, wanted to make it hard-fail so it easy to spot when it happens
> > > again. Although it might be cases where the module is built-in this is
> > > not the case in CI.
> > 
> > The || SKIP was added in
> > 
> > commit f14d56c42d9e43df2790465aba6a2ea2593418fc
> > Author: Chris Wilson <chris at chris-wilson.co.uk>
> > Date:   Fri Mar 11 21:25:48 2016 +0000
> > 
> >     igt/drv_module_reload_basic: Rinse and repeat with addition module parameters
> > 
> > and apparently not intentionally.
> 
> It was, because the test wasn't behaving properly on my setup where I
> had disabled module unloading. If we can't unload the module, we have to
> SKIP the test.

The problem is that we actually have the test split up incorrectly.
Instead of doing a

preparation phase;

for_each_subtest
	load;
	check;

	unload
	check;

restoration

we have 

for_each_subtest;
	unload
	load
	check

and so we are doingthe tail of the first test in the second.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre