Re: [PATCH v3] drm/i915: Add fault injection support

[Imre Deak <imre.deak@intel.com>]

On Wed, 2016-03-16 at 10:04 +0000, Chris Wilson wrote:
> On Wed, Mar 16, 2016 at 11:43:14AM +0200, Imre Deak wrote:
> > On Wed, 2016-03-16 at 09:24 +0000, Chris Wilson wrote:
> > > On Wed, Mar 16, 2016 at 11:18:04AM +0200, Joonas Lahtinen wrote:
> > > > On ti, 2016-03-15 at 14:14 +0000, Chris Wilson wrote:
> > > > > On Tue, Mar 15, 2016 at 04:01:14PM +0200, Imre Deak wrote:
> > > > > > 
> > > > > > I'm not sure if you want to check all failure paths, I
> > > > > > think
> > > > > > for that
> > > > > > the existing failslab etc. mechanisms are better suited.
> > > > > > This
> > > > > > new
> > > > > > option would be used at relatively few well defined places.
> > > > > > The
> > > > > > option
> > > > > > is a mask since Chris wanted the possibility to mix
> > > > > > failures
> > > > > > (which
> > > > > > makes sense when injecting a non-fatal failure somewhere).
> > > > > > If
> > > > > > he
> > > > > > doesn't insist on that possibility I can convert the mask
> > > > > > option to a
> > > > > > counter based one identifying a single failure spot instead
> > > > > > as
> > > > > > you
> > > > > > suggest. Chris?
> > > > > We can extend the counter mechanism by having multiple
> > > > > counters
> > > > > behind
> > > > > i915.inject_load_failure (i.e. =gem:4,driver:10,modeset:1)
> > > > 
> > > > Now that there's a series to split down the init functions
> > > > nicely,
> > > > one
> > > > could use the function names directly. By stripping parts of it
> > > > if
> > > > needed to shorten them.
> > > 
> > > That's nice. Advantage for using counters is that we can write a
> > > test
> > > to
> > > iterate over the first thousand and have it run against future
> > > faultpointers. Advantage for using names is that it is readable
> > > and
> > > easily extensible.
> > > 
> > > What we could do for testing is record the available fault
> > > injection
> > > points for debugfs and so have the tests automatically adjust
> > > (given
> > > a
> > > working start point). Overkill?
> > 
> > I'd prefer if we could fine grain things once we have more failure
> > points. Atm the ones we defined cause driver init to fail, so
> > separate 
> > counters wouldn't bring us much. I suggest that we keep the module
> > option as a simple integer for now and extend it later to support
> > the
> > above tagged counter mechanism and debugfs interface. Here is what
> > I
> > planned to submit to the list:
> > 
> > https://github.com/ideak/linux/commits/driver_init_refactor
> 
> Move the inline into __i915_inject_load_failure

hm, I have the macro in the header file to optimize for the
failure_count=0 case. Or did you mean to
bring __i915_inject_load_failure to the header file too/merge it into
the macro?

> , and yes I am happy with
> the simple first step.  Please use __func__ or __FUNCTION__ though as
> that is more descriptive than file, especially when they move.

Ok.

--Imre

> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx