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@xxxxxxxxx> > > > > --- > > > > 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@xxxxxxxxxxxxxxxxxx> 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. Please cite that commit using Fixes: and ack on your patch from my side. -Daniel > > > > > Continuing on after failure to unload is not a good idea. > > -Chris > > > > -- > > Chris Wilson, Intel Open Source Technology Centre -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx