| Message ID | 20200706140125.172844-3-michal@hardline.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/3] drm/i915: Reboot CI if we get wedged during driver init | expand |
Quoting Michał Winiarski (2020-07-06 15:01:25) > From: Michał Winiarski <michal.winiarski@intel.com> > > It is not really unexpected to hit wedge on init this way. > We're already downgrading error printk when running with fault injection, > let's use the same approach for CI tainting. > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Petri Latvala <petri.latvala@intel.com> > --- > drivers/gpu/drm/i915/i915_utils.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h > index 82fada1e7552..d84c23592942 100644 > --- a/drivers/gpu/drm/i915/i915_utils.h > +++ b/drivers/gpu/drm/i915/i915_utils.h > @@ -438,7 +438,9 @@ static inline const char *enableddisabled(bool v) > > static inline void __add_taint_for_CI(unsigned int taint) > { > - add_taint(taint, LOCKDEP_STILL_OK); > + /* Failures that occur during fault injection testing are expected */ > + if (!i915_error_injected()) > + add_taint(taint, LOCKDEP_STILL_OK); > } And I think this is better in add_taint_for_CI(). If we hit the GEM_BUG_ON() we should always add the taint, as that trace dump is a one-shot affair. -Chris
Quoting Chris Wilson (2020-07-06 15:10:02) > Quoting Michał Winiarski (2020-07-06 15:01:25) > > From: Michał Winiarski <michal.winiarski@intel.com> > > > > It is not really unexpected to hit wedge on init this way. > > We're already downgrading error printk when running with fault injection, > > let's use the same approach for CI tainting. > > > > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Cc: Petri Latvala <petri.latvala@intel.com> > > --- > > drivers/gpu/drm/i915/i915_utils.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h > > index 82fada1e7552..d84c23592942 100644 > > --- a/drivers/gpu/drm/i915/i915_utils.h > > +++ b/drivers/gpu/drm/i915/i915_utils.h > > @@ -438,7 +438,9 @@ static inline const char *enableddisabled(bool v) > > > > static inline void __add_taint_for_CI(unsigned int taint) > > { > > - add_taint(taint, LOCKDEP_STILL_OK); > > + /* Failures that occur during fault injection testing are expected */ > > + if (!i915_error_injected()) > > + add_taint(taint, LOCKDEP_STILL_OK); > > } > > And I think this is better in add_taint_for_CI(). If we hit the > GEM_BUG_ON() we should always add the taint, as that trace dump is a > one-shot affair. So if you agree, make add_taint_for_CI out-of-line and it there, and you can have a Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> straight away :) -Chris
diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h index 82fada1e7552..d84c23592942 100644 --- a/drivers/gpu/drm/i915/i915_utils.h +++ b/drivers/gpu/drm/i915/i915_utils.h @@ -438,7 +438,9 @@ static inline const char *enableddisabled(bool v) static inline void __add_taint_for_CI(unsigned int taint) { - add_taint(taint, LOCKDEP_STILL_OK); + /* Failures that occur during fault injection testing are expected */ + if (!i915_error_injected()) + add_taint(taint, LOCKDEP_STILL_OK); } static inline void