| Message ID | 20170410121747.209200-1-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 10, 2017 at 12:17:47PM +0000, Michal Wajdeczko wrote: > This function should not be called with long timeouts in atomic context. > Annotate it as might_sleep if timeout is longer than 10us. > > v2: fix comment (Michal) > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> And pushed, thanks for the patch and for including the limits in the doc. -Chris
HI, > -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf > Of Chris Wilson > Sent: Monday, April 10, 2017 5:26 PM > To: Wajdeczko, Michal <Michal.Wajdeczko@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Don't allow overuse of > __intel_wait_for_register_fw() > > On Mon, Apr 10, 2017 at 12:17:47PM +0000, Michal Wajdeczko wrote: > > This function should not be called with long timeouts in atomic context. > > Annotate it as might_sleep if timeout is longer than 10us. > > > > v2: fix comment (Michal) > > > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > And pushed, thanks for the patch and for including the limits in the doc. > -Chris Was there reason why pushed as clearly seen issues on pw run: https://patchwork.freedesktop.org/series/22774/ Or is there known issues we are planning to fix now? Now same faults on on tip: https://intel-gfx-ci.01.org/CI/ Br, Jani Saarinen Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
On Mon, Apr 10, 2017 at 03:26:06PM +0100, Chris Wilson wrote: > On Mon, Apr 10, 2017 at 12:17:47PM +0000, Michal Wajdeczko wrote: > > This function should not be called with long timeouts in atomic context. > > Annotate it as might_sleep if timeout is longer than 10us. > > > > v2: fix comment (Michal) > > > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > And pushed, thanks for the patch and for including the limits in the > doc. Drat, skimmed over the warning from patchwork. Just didn't believe we had that latent bug... -Chris
On Mon, Apr 10, 2017 at 02:58:34PM +0000, Saarinen, Jani wrote: > HI, > > -----Original Message----- > > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf > > Of Chris Wilson > > Sent: Monday, April 10, 2017 5:26 PM > > To: Wajdeczko, Michal <Michal.Wajdeczko@intel.com> > > Cc: intel-gfx@lists.freedesktop.org > > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Don't allow overuse of > > __intel_wait_for_register_fw() > > > > On Mon, Apr 10, 2017 at 12:17:47PM +0000, Michal Wajdeczko wrote: > > > This function should not be called with long timeouts in atomic context. > > > Annotate it as might_sleep if timeout is longer than 10us. > > > > > > v2: fix comment (Michal) > > > > > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > > > And pushed, thanks for the patch and for including the limits in the doc. > > -Chris > Was there reason why pushed as clearly seen issues on pw run: https://patchwork.freedesktop.org/series/22774/ > Or is there known issues we are planning to fix now? Now same faults on on tip: https://intel-gfx-ci.01.org/CI/ Because I was blind. The bug was latent and fix is sent. -Chris
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 1deb1a4..eb38392 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1600,6 +1600,8 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv, * (I915_READ_FW(reg) & mask) == value * * Otherwise, the wait will timeout after @slow_timeout_ms milliseconds. + * For atomic context @slow_timeout_ms must be zero and @fast_timeout_us + * must be not larger than 10 microseconds. * * Note that this routine assumes the caller holds forcewake asserted, it is * not suitable for very long waits. See intel_wait_for_register() if you @@ -1620,6 +1622,9 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv, #define done (((reg_value = I915_READ_FW(reg)) & mask) == value) int ret; + /* Catch any overuse of this function */ + might_sleep_if(fast_timeout_us > 10 || slow_timeout_ms); + if (fast_timeout_us > 10) ret = _wait_for(done, fast_timeout_us, 10); else
This function should not be called with long timeouts in atomic context. Annotate it as might_sleep if timeout is longer than 10us. v2: fix comment (Michal) Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_uncore.c | 5 +++++ 1 file changed, 5 insertions(+)