[igt-dev] [PATCH i-g-t v2] igt/drv_module_reload: Revamp fault-injection

[Imre Deak]

On Wed, Jun 06, 2018 at 07:04:47PM +0100, Chris Wilson wrote:
> Quoting Imre Deak (2018-06-06 19:00:52)
> > On Wed, Jun 06, 2018 at 06:42:14PM +0100, Chris Wilson wrote:
> > > The current method of checking for a failed module load is flawed, as we
> > > only report the error on probing it is not being reported back by
> > > modprobe. So we have to dig inside the module_parameters while the
> > > module is still loaded to discover the error.
> > > 
> > > v2: Expect i915.inject_load_failure to be zero on success
> > > 
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Michał Winiarski <michal.winiarski at intel.com>
> > > Cc: Imre Deak <imre.deak at intel.com>
> > > Reviewed-by: Michał Winiarski <michal.winiarski at intel.com>
> > > ---
> > >  tests/drv_module_reload.c | 45 ++++++++++++++++++++++++++++++++++-----
> > >  1 file changed, 40 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
> > > index 092982960..e18aaea5e 100644
> > > --- a/tests/drv_module_reload.c
> > > +++ b/tests/drv_module_reload.c
> > > @@ -234,6 +234,38 @@ reload(const char *opts_i915)
> > >       return err;
> > >  }
> > >  
> > > +static int open_parameters(const char *module_name)
> > > +{
> > > +     char path[256];
> > > +
> > > +     snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> > > +     return open(path, O_RDONLY);
> > > +}
> > > +
> > > +static int
> > > +inject_fault(const char *module_name, const char *opt, int fault)
> > > +{
> > > +     char buf[1024];
> > > +     int dir;
> > > +
> > > +     igt_assert(fault > 0);
> > > +     snprintf(buf, sizeof(buf), "%s=%d", opt, fault);
> > > +
> > > +     if (igt_kmod_load(module_name, buf)) {
> > > +             igt_warn("Failed to load module '%s' with options '%s'\n",
> > > +                      module_name, buf);
> > > +             return 1;
> > > +     }
> > > +
> > > +     dir = open_parameters(module_name);
> > > +     igt_sysfs_scanf(dir, opt, "%d", &fault);
> > 
> > Just a note for later: in case we switch to async probing we'll have to
> > wait somehow until the probe completed. One way would be to disable
> > async probing for this test, not sure if that could hide some problem
> > though.
> 
> modprobe already waits for async probes (via async_synchronize_full()
> before it returns to userspace). Afaict, that async flag only has any
> relevance for builtins. If you want to parallel module loading, you need
> to fork.

Hm, right. Looking at that now, there's also the async_probe module
param, but we only need to care about the driver flag. So looks ok then.

> -Chris