diff mbox

drm/i915: Mark i915.inject_load_failure as being hit

Message ID 20180606130937.20771-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson June 6, 2018, 1:09 p.m. UTC
When we reach the magic value and do inject a fault into our module load,
mark the module option as being hit. Since we fail from inside pci
probe, the module load isn't actually aborted and the module (and
paramters) are left lingering. igt can then inspect the parameter on its
synchronous completion of modprobe to see if the fault injection was
successful, and will keeping on injecting new faults until the module
succeeds in loading having surpassed the number of fault points.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c    | 1 +
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Michal Wajdeczko June 6, 2018, 1:25 p.m. UTC | #1
On Wed, 06 Jun 2018 15:09:37 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> When we reach the magic value and do inject a fault into our module load,
> mark the module option as being hit. Since we fail from inside pci
> probe, the module load isn't actually aborted and the module (and
> paramters) are left lingering. igt can then inspect the parameter on its

typo

> synchronous completion of modprobe to see if the fault injection was
> successful, and will keeping on injecting new faults until the module
> succeeds in loading having surpassed the number of fault points.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c    | 1 +
>  drivers/gpu/drm/i915/i915_params.h | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c  
> b/drivers/gpu/drm/i915/i915_drv.c
> index 6b599b1837dc..71b7302e6f8c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -67,6 +67,7 @@ bool __i915_inject_load_failure(const char *func, int  
> line)
>  	if (++i915_load_fail_count == i915_modparams.inject_load_failure) {
>  		DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
>  			 i915_modparams.inject_load_failure, func, line);
> +		i915_modparams.inject_load_failure = -1;

maybe we can just reset inject_load_failure to 0 then we can
avoid changing modparam type and/or use -1 for other purposes,
like proposed in [1]

Michal

[1] https://patchwork.freedesktop.org/patch/201798/

>  		return true;
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_params.h  
> b/drivers/gpu/drm/i915/i915_params.h
> index aebe0469ddaa..3b115975745a 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -55,7 +55,7 @@ struct drm_printer;
>  	param(int, mmio_debug, 0) \
>  	param(int, edp_vswing, 0) \
>  	param(int, reset, 2) \
> -	param(unsigned int, inject_load_failure, 0) \
> +	param(int, inject_load_failure, 0) \
>  	/* leave bools at the end to not create holes */ \
>  	param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
>  	param(bool, enable_hangcheck, true) \
Chris Wilson June 6, 2018, 1:33 p.m. UTC | #2
Quoting Michal Wajdeczko (2018-06-06 14:25:34)
> On Wed, 06 Jun 2018 15:09:37 +0200, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > When we reach the magic value and do inject a fault into our module load,
> > mark the module option as being hit. Since we fail from inside pci
> > probe, the module load isn't actually aborted and the module (and
> > paramters) are left lingering. igt can then inspect the parameter on its
> 
> typo
> 
> > synchronous completion of modprobe to see if the fault injection was
> > successful, and will keeping on injecting new faults until the module
> > succeeds in loading having surpassed the number of fault points.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c    | 1 +
> >  drivers/gpu/drm/i915/i915_params.h | 2 +-
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c  
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 6b599b1837dc..71b7302e6f8c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -67,6 +67,7 @@ bool __i915_inject_load_failure(const char *func, int  
> > line)
> >       if (++i915_load_fail_count == i915_modparams.inject_load_failure) {
> >               DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
> >                        i915_modparams.inject_load_failure, func, line);
> > +             i915_modparams.inject_load_failure = -1;
> 
> maybe we can just reset inject_load_failure to 0 then we can
> avoid changing modparam type and/or use -1 for other purposes,
> like proposed in [1]

0 would be fine for my purpose.
-Chris
Chris Wilson June 6, 2018, 2:19 p.m. UTC | #3
Quoting Chris Wilson (2018-06-06 14:33:19)
> Quoting Michal Wajdeczko (2018-06-06 14:25:34)
> > On Wed, 06 Jun 2018 15:09:37 +0200, Chris Wilson  
> > <chris@chris-wilson.co.uk> wrote:
> > 
> > > When we reach the magic value and do inject a fault into our module load,
> > > mark the module option as being hit. Since we fail from inside pci
> > > probe, the module load isn't actually aborted and the module (and
> > > paramters) are left lingering. igt can then inspect the parameter on its
> > 
> > typo
> > 
> > > synchronous completion of modprobe to see if the fault injection was
> > > successful, and will keeping on injecting new faults until the module
> > > succeeds in loading having surpassed the number of fault points.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Michał Winiarski <michal.winiarski@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c    | 1 +
> > >  drivers/gpu/drm/i915/i915_params.h | 2 +-
> > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c  
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 6b599b1837dc..71b7302e6f8c 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -67,6 +67,7 @@ bool __i915_inject_load_failure(const char *func, int  
> > > line)
> > >       if (++i915_load_fail_count == i915_modparams.inject_load_failure) {
> > >               DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
> > >                        i915_modparams.inject_load_failure, func, line);
> > > +             i915_modparams.inject_load_failure = -1;
> > 
> > maybe we can just reset inject_load_failure to 0 then we can
> > avoid changing modparam type and/or use -1 for other purposes,
> > like proposed in [1]
> 
> 0 would be fine for my purpose.

Except i915_error_injected() would suite -1 better. Different plan for
that one then.
-Chris
Michal Wajdeczko June 6, 2018, 2:28 p.m. UTC | #4
On Wed, 06 Jun 2018 16:19:29 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Chris Wilson (2018-06-06 14:33:19)
>> Quoting Michal Wajdeczko (2018-06-06 14:25:34)
>> > On Wed, 06 Jun 2018 15:09:37 +0200, Chris Wilson
>> > <chris@chris-wilson.co.uk> wrote:
>> >
>> > > When we reach the magic value and do inject a fault into our module  
>> load,
>> > > mark the module option as being hit. Since we fail from inside pci
>> > > probe, the module load isn't actually aborted and the module (and
>> > > paramters) are left lingering. igt can then inspect the parameter  
>> on its
>> >
>> > typo
>> >
>> > > synchronous completion of modprobe to see if the fault injection was
>> > > successful, and will keeping on injecting new faults until the  
>> module
>> > > succeeds in loading having surpassed the number of fault points.
>> > >
>> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > > Cc: Michał Winiarski <michal.winiarski@intel.com>
>> > > Cc: Imre Deak <imre.deak@intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/i915_drv.c    | 1 +
>> > >  drivers/gpu/drm/i915/i915_params.h | 2 +-
>> > >  2 files changed, 2 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> > > b/drivers/gpu/drm/i915/i915_drv.c
>> > > index 6b599b1837dc..71b7302e6f8c 100644
>> > > --- a/drivers/gpu/drm/i915/i915_drv.c
>> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
>> > > @@ -67,6 +67,7 @@ bool __i915_inject_load_failure(const char *func,  
>> int
>> > > line)
>> > >       if (++i915_load_fail_count ==  
>> i915_modparams.inject_load_failure) {
>> > >               DRM_INFO("Injecting failure at checkpoint %u  
>> [%s:%d]\n",
>> > >                        i915_modparams.inject_load_failure, func,  
>> line);
>> > > +             i915_modparams.inject_load_failure = -1;
>> >
>> > maybe we can just reset inject_load_failure to 0 then we can
>> > avoid changing modparam type and/or use -1 for other purposes,
>> > like proposed in [1]
>>
>> 0 would be fine for my purpose.
>
> Except i915_error_injected() would suite -1 better. Different plan for
> that one then.

but you can easily fix that:

  static bool i915_error_injected(struct drm_i915_private *dev_priv)
  {
  #if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
-       return i915_modparams.inject_load_failure &&
-              i915_load_fail_count == i915_modparams.inject_load_failure;
+       return i915_load_fail_count && !i915_modparams.inject_load_failure;
  #else
         return false;
  #endif
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6b599b1837dc..71b7302e6f8c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -67,6 +67,7 @@  bool __i915_inject_load_failure(const char *func, int line)
 	if (++i915_load_fail_count == i915_modparams.inject_load_failure) {
 		DRM_INFO("Injecting failure at checkpoint %u [%s:%d]\n",
 			 i915_modparams.inject_load_failure, func, line);
+		i915_modparams.inject_load_failure = -1;
 		return true;
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index aebe0469ddaa..3b115975745a 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -55,7 +55,7 @@  struct drm_printer;
 	param(int, mmio_debug, 0) \
 	param(int, edp_vswing, 0) \
 	param(int, reset, 2) \
-	param(unsigned int, inject_load_failure, 0) \
+	param(int, inject_load_failure, 0) \
 	/* leave bools at the end to not create holes */ \
 	param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
 	param(bool, enable_hangcheck, true) \