[PATCH] drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering

[PATCH] drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering

[Chris Wilson]

Since the removal of the stop_machine(), it is allowed and expected for
the nop_submit_request() and nop_complete_submit_request() to run in
parallel to the i915_gem_set_wedged() processing. As such we can no
longer assert that i915_gem_set_wedged() has completed inside the
stop_machine prior to the individual nop_submit_request execution.

Fixes: af7a8ffad9c5 ("drm/i915: Use rcu instead of stop_machine in set_wedged")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 20fcac37c85a..d9d39b309ce8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3087,7 +3087,6 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
 
 static void nop_submit_request(struct drm_i915_gem_request *request)
 {
-	GEM_BUG_ON(!i915_terminally_wedged(&request->i915->gpu_error));
 	dma_fence_set_error(&request->fence, -EIO);
 
 	i915_gem_request_submit(request);
@@ -3097,7 +3096,6 @@ static void nop_complete_submit_request(struct drm_i915_gem_request *request)
 {
 	unsigned long flags;
 
-	GEM_BUG_ON(!i915_terminally_wedged(&request->i915->gpu_error));
 	dma_fence_set_error(&request->fence, -EIO);
 
 	spin_lock_irqsave(&request->engine->timeline->lock, flags);
-- 
2.15.0.rc0

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

✗ Fi.CI.BAT: failure for drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering

[Patchwork]

== Series Details ==

Series: drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering
URL   : https://patchwork.freedesktop.org/series/31848/
State : failure

== Summary ==

Series 31848v1 drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering
https://patchwork.freedesktop.org/api/1.0/series/31848/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test kms_cursor_legacy:
        Subgroup basic-flip-before-cursor-varying-size:
                pass       -> DMESG-FAIL (fi-elk-e7500)
Test drv_module_reload:
        Subgroup basic-no-display:
                dmesg-warn -> INCOMPLETE (fi-cfl-s) fdo#103206

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#103206 https://bugs.freedesktop.org/show_bug.cgi?id=103206

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:455s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:470s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:392s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:567s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:287s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:530s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:521s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:536s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:524s
fi-cfl-s         total:287  pass:253  dwarn:2   dfail:0   fail:0   skip:31 
fi-elk-e7500     total:289  pass:228  dwarn:0   dfail:1   fail:0   skip:60  time:533s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:276s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:600s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:439s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:462s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:494s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:473s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:504s
fi-kbl-7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  time:496s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:592s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:664s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:469s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:660s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:536s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:561s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:470s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:588s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:434s

490bd134d63554815d2ea6ff56e6d32fd50ff472 drm-tip: 2017y-10m-12d-20h-08m-10s UTC integration manifest
3f7844afb0ba drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6015/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since the removal of the stop_machine(), it is allowed and expected for
> the nop_submit_request() and nop_complete_submit_request() to run in
> parallel to the i915_gem_set_wedged() processing. As such we can no
> longer assert that i915_gem_set_wedged() has completed inside the
> stop_machine prior to the individual nop_submit_request execution.
>
> Fixes: af7a8ffad9c5 ("drm/i915: Use rcu instead of stop_machine in set_wedged")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

from irc:
17:12 < danvet> r-b: me

also,

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 20fcac37c85a..d9d39b309ce8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3087,7 +3087,6 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
>  
>  static void nop_submit_request(struct drm_i915_gem_request *request)
>  {
> -	GEM_BUG_ON(!i915_terminally_wedged(&request->i915->gpu_error));
>  	dma_fence_set_error(&request->fence, -EIO);
>  
>  	i915_gem_request_submit(request);
> @@ -3097,7 +3096,6 @@ static void nop_complete_submit_request(struct drm_i915_gem_request *request)
>  {
>  	unsigned long flags;
>  
> -	GEM_BUG_ON(!i915_terminally_wedged(&request->i915->gpu_error));
>  	dma_fence_set_error(&request->fence, -EIO);
>  
>  	spin_lock_irqsave(&request->engine->timeline->lock, flags);
> -- 
> 2.15.0.rc0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering

[Chris Wilson]

Quoting Mika Kuoppala (2017-10-13 15:23:41)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Since the removal of the stop_machine(), it is allowed and expected for
> > the nop_submit_request() and nop_complete_submit_request() to run in
> > parallel to the i915_gem_set_wedged() processing. As such we can no
> > longer assert that i915_gem_set_wedged() has completed inside the
> > stop_machine prior to the individual nop_submit_request execution.
> >
> > Fixes: af7a8ffad9c5 ("drm/i915: Use rcu instead of stop_machine in set_wedged")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> from irc:
> 17:12 < danvet> r-b: me
> 
> also,
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Just a note to say we could, move the set_bit(WEDGED) first (followed by
a smb_mb__after_atomic()) so that the concurrent nop_submit_request
would see the right bit. However, moving that bit requires a bit more
thought wrt to the all the users and what it means for
i915_gem_set_wedged(). Simpler just to remove the incorrect BUG_ON for
now and address again in future.

The other challenge is hitting this race in testing. We have to
coordinate the requests becoming ready in parallel to the failed
reset. Something like queueing 100,000 requests and signaling them at
intervals of a couple of micros-econds would do the trick. That itself
is not too difficult, the remaining challenge will be in coordinating
that with the reset -- so using hangcheck is out the window and we must
trigger the EIO directly. Sounds easy!

Thanks for the review,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Stop asserting on set-wedged vs nop_submit_request ordering

[Chris Wilson]

Quoting Mika Kuoppala (2017-10-13 15:23:41)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Since the removal of the stop_machine(), it is allowed and expected for
> > the nop_submit_request() and nop_complete_submit_request() to run in
> > parallel to the i915_gem_set_wedged() processing. As such we can no
> > longer assert that i915_gem_set_wedged() has completed inside the
> > stop_machine prior to the individual nop_submit_request execution.
> >
> > Fixes: af7a8ffad9c5 ("drm/i915: Use rcu instead of stop_machine in set_wedged")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> from irc:
> 17:12 < danvet> r-b: me
> 
> also,
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Applied but it didn't fix snb; it still dies silently in
gem_eio/suspend. I knew it was a long shot, but the only one I had. :|

If CI isn't giving us the info we need, I guess I'll just have to try
locally.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx