All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request
@ 2017-03-08 22:08 Chris Wilson
  2017-03-09  0:17 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Chris Wilson @ 2017-03-08 22:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: intel-gvt-dev

commit 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly")
includes some nonsense to retry a indefinite wait - i915_wait_request()
does not return until the request is completed when used from an
uninterruptible context.

Fixes: 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly"
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Cc: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/gvt/scheduler.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index cfd9f7a38c27..8024904c0d58 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -465,19 +465,7 @@ static int workload_thread(void *priv)
 
 		gvt_dbg_sched("ring id %d wait workload %p\n",
 				workload->ring_id, workload);
-retry:
-		i915_wait_request(workload->req,
-					 0, MAX_SCHEDULE_TIMEOUT);
-		/* I915 has replay mechanism and a request will be replayed
-		 * if there is i915 reset. So the seqno will be updated anyway.
-		 * If the seqno is not updated yet after waiting, which means
-		 * the replay may still be in progress and we can wait again.
-		 */
-		if (!i915_gem_request_completed(workload->req)) {
-			gvt_dbg_sched("workload %p not completed, wait again\n",
-					workload);
-			goto retry;
-		}
+		i915_wait_request(workload->req, 0, MAX_SCHEDULE_TIMEOUT);
 
 complete:
 		gvt_dbg_sched("will complete workload %p, status: %d\n",
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/gvt: Remove bogus retry around i915_wait_request
  2017-03-08 22:08 [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request Chris Wilson
@ 2017-03-09  0:17 ` Patchwork
  2017-03-09  1:28 ` [PATCH] " Dong, Chuanxiao
  2017-03-13  9:40 ` Zhenyu Wang
  2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2017-03-09  0:17 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gvt: Remove bogus retry around i915_wait_request
URL   : https://patchwork.freedesktop.org/series/20947/
State : success

== Summary ==

Series 20947v1 drm/i915/gvt: Remove bogus retry around i915_wait_request
https://patchwork.freedesktop.org/api/1.0/series/20947/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                dmesg-warn -> PASS       (fi-bxt-t5700) fdo#100125

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

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 463s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 614s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 543s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 608s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 509s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 499s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 443s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 441s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 507s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 496s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 480s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 501s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 603s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 502s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 548s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 560s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 414s

226d87634d5f7aa9104d1829c7ef7f7133d94e39 drm-tip: 2017y-03m-08d-22h-43m-24s UTC integration manifest
eb73ea5 drm/i915/gvt: Remove bogus retry around i915_wait_request

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4107/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request
  2017-03-08 22:08 [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request Chris Wilson
  2017-03-09  0:17 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-03-09  1:28 ` Dong, Chuanxiao
  2017-03-13  9:21   ` Dong, Chuanxiao
  2017-03-13  9:40 ` Zhenyu Wang
  2 siblings, 1 reply; 7+ messages in thread
From: Dong, Chuanxiao @ 2017-03-09  1:28 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: intel-gvt-dev



> -----Original Message-----
> From: Chris Wilson [mailto:chris.ickle.wilson@gmail.com] On Behalf Of Chris
> Wilson
> Sent: Thursday, March 9, 2017 6:08 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org; Chris Wilson <chris@chris-
> wilson.co.uk>; Dong, Chuanxiao <chuanxiao.dong@intel.com>; Zhenyu
> Wang <zhenyuw@linux.intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: [PATCH] drm/i915/gvt: Remove bogus retry around
> i915_wait_request
> 
> commit 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly")
> includes some nonsense to retry a indefinite wait - i915_wait_request() does
> not return until the request is completed when used from an uninterruptible
> context.
> 
> Fixes: 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly"
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/scheduler.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index cfd9f7a38c27..8024904c0d58 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -465,19 +465,7 @@ static int workload_thread(void *priv)
> 
>  		gvt_dbg_sched("ring id %d wait workload %p\n",
>  				workload->ring_id, workload);
> -retry:
> -		i915_wait_request(workload->req,
> -					 0, MAX_SCHEDULE_TIMEOUT);
> -		/* I915 has replay mechanism and a request will be replayed
> -		 * if there is i915 reset. So the seqno will be updated anyway.
> -		 * If the seqno is not updated yet after waiting, which means
> -		 * the replay may still be in progress and we can wait again.
> -		 */
> -		if (!i915_gem_request_completed(workload->req)) {
> -			gvt_dbg_sched("workload %p not completed, wait
> again\n",
> -					workload);
> -			goto retry;
> -		}
> +		i915_wait_request(workload->req, 0,
> MAX_SCHEDULE_TIMEOUT);

Thanks Chris for pointing this out. So per the comment, i915_wait_request will make sure to return until the request completed, even if a GPU hang happened and replay occurred during i915_wait_request(), this function will only return after the replay completed, right?

> 
>  complete:
>  		gvt_dbg_sched("will complete workload %p, status: %d\n",
> --
> 2.11.0

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request
  2017-03-09  1:28 ` [PATCH] " Dong, Chuanxiao
@ 2017-03-13  9:21   ` Dong, Chuanxiao
  2017-03-13  9:28     ` Chris Wilson
  0 siblings, 1 reply; 7+ messages in thread
From: Dong, Chuanxiao @ 2017-03-13  9:21 UTC (permalink / raw)
  To: Dong, Chuanxiao, Chris Wilson, intel-gfx; +Cc: intel-gvt-dev



> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Dong, Chuanxiao
> Sent: Thursday, March 9, 2017 9:28 AM
> To: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A
> <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
> Subject: RE: [PATCH] drm/i915/gvt: Remove bogus retry around
> i915_wait_request
> 
> 
> 
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris.ickle.wilson@gmail.com] On Behalf Of
> > Chris Wilson
> > Sent: Thursday, March 9, 2017 6:08 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: intel-gvt-dev@lists.freedesktop.org; Chris Wilson <chris@chris-
> > wilson.co.uk>; Dong, Chuanxiao <chuanxiao.dong@intel.com>; Zhenyu
> Wang
> > <zhenyuw@linux.intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > Subject: [PATCH] drm/i915/gvt: Remove bogus retry around
> > i915_wait_request
> >
> > commit 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle
> > properly") includes some nonsense to retry a indefinite wait -
> > i915_wait_request() does not return until the request is completed
> > when used from an uninterruptible context.
> >
> > Fixes: 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly"
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Cc: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/scheduler.c | 14 +-------------
> >  1 file changed, 1 insertion(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > index cfd9f7a38c27..8024904c0d58 100644
> > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > @@ -465,19 +465,7 @@ static int workload_thread(void *priv)
> >
> >  		gvt_dbg_sched("ring id %d wait workload %p\n",
> >  				workload->ring_id, workload);
> > -retry:
> > -		i915_wait_request(workload->req,
> > -					 0, MAX_SCHEDULE_TIMEOUT);
> > -		/* I915 has replay mechanism and a request will be replayed
> > -		 * if there is i915 reset. So the seqno will be updated anyway.
> > -		 * If the seqno is not updated yet after waiting, which means
> > -		 * the replay may still be in progress and we can wait again.
> > -		 */
> > -		if (!i915_gem_request_completed(workload->req)) {
> > -			gvt_dbg_sched("workload %p not completed, wait
> > again\n",
> > -					workload);
> > -			goto retry;
> > -		}
> > +		i915_wait_request(workload->req, 0,
> > MAX_SCHEDULE_TIMEOUT);
> 
> Thanks Chris for pointing this out. So per the comment, i915_wait_request
> will make sure to return until the request completed, even if a GPU hang
> happened and replay occurred during i915_wait_request(), this function will
> only return after the replay completed, right?

Hi Chris, ping for the above question. I guess the answer is "yes"?

Thanks
Chuanxiao

> 
> >
> >  complete:
> >  		gvt_dbg_sched("will complete workload %p, status: %d\n",
> > --
> > 2.11.0
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request
  2017-03-13  9:21   ` Dong, Chuanxiao
@ 2017-03-13  9:28     ` Chris Wilson
  2017-03-13  9:35       ` Dong, Chuanxiao
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2017-03-13  9:28 UTC (permalink / raw)
  To: Dong, Chuanxiao; +Cc: intel-gfx, intel-gvt-dev

On Mon, Mar 13, 2017 at 09:21:58AM +0000, Dong, Chuanxiao wrote:
> 
> 
> > -----Original Message-----
> > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> > Behalf Of Dong, Chuanxiao
> > Sent: Thursday, March 9, 2017 9:28 AM
> > To: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org
> > Cc: intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A
> > <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
> > Subject: RE: [PATCH] drm/i915/gvt: Remove bogus retry around
> > i915_wait_request
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris.ickle.wilson@gmail.com] On Behalf Of
> > > Chris Wilson
> > > Sent: Thursday, March 9, 2017 6:08 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: intel-gvt-dev@lists.freedesktop.org; Chris Wilson <chris@chris-
> > > wilson.co.uk>; Dong, Chuanxiao <chuanxiao.dong@intel.com>; Zhenyu
> > Wang
> > > <zhenyuw@linux.intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > > Subject: [PATCH] drm/i915/gvt: Remove bogus retry around
> > > i915_wait_request
> > >
> > > commit 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle
> > > properly") includes some nonsense to retry a indefinite wait -
> > > i915_wait_request() does not return until the request is completed
> > > when used from an uninterruptible context.
> > >
> > > Fixes: 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly"
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Cc: Zhi Wang <zhi.a.wang@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/scheduler.c | 14 +-------------
> > >  1 file changed, 1 insertion(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > index cfd9f7a38c27..8024904c0d58 100644
> > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > @@ -465,19 +465,7 @@ static int workload_thread(void *priv)
> > >
> > >  		gvt_dbg_sched("ring id %d wait workload %p\n",
> > >  				workload->ring_id, workload);
> > > -retry:
> > > -		i915_wait_request(workload->req,
> > > -					 0, MAX_SCHEDULE_TIMEOUT);
> > > -		/* I915 has replay mechanism and a request will be replayed
> > > -		 * if there is i915 reset. So the seqno will be updated anyway.
> > > -		 * If the seqno is not updated yet after waiting, which means
> > > -		 * the replay may still be in progress and we can wait again.
> > > -		 */
> > > -		if (!i915_gem_request_completed(workload->req)) {
> > > -			gvt_dbg_sched("workload %p not completed, wait
> > > again\n",
> > > -					workload);
> > > -			goto retry;
> > > -		}
> > > +		i915_wait_request(workload->req, 0,
> > > MAX_SCHEDULE_TIMEOUT);
> > 
> > Thanks Chris for pointing this out. So per the comment, i915_wait_request
> > will make sure to return until the request completed, even if a GPU hang
> > happened and replay occurred during i915_wait_request(), this function will
> > only return after the replay completed, right?
> 
> Hi Chris, ping for the above question. I guess the answer is "yes"?

Yes. Given MAX_SCHEDULE_TIMEOUT, no INTERRUPTIBLE flags, it will only
return once the request is completed - that is the hw breadcrumb is
advanced past the request seqno. That includes waiting across a GPU
reset. It is not allowed to return another error.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request
  2017-03-13  9:28     ` Chris Wilson
@ 2017-03-13  9:35       ` Dong, Chuanxiao
  0 siblings, 0 replies; 7+ messages in thread
From: Dong, Chuanxiao @ 2017-03-13  9:35 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, intel-gvt-dev



> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Chris Wilson
> Sent: Monday, March 13, 2017 5:28 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
> Zhenyu Wang <zhenyuw@linux.intel.com>; Wang, Zhi A
> <zhi.a.wang@intel.com>
> Subject: Re: [PATCH] drm/i915/gvt: Remove bogus retry around
> i915_wait_request
> 
> On Mon, Mar 13, 2017 at 09:21:58AM +0000, Dong, Chuanxiao wrote:
> >
> >
> > > -----Original Message-----
> > > From: intel-gvt-dev
> > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > > Dong, Chuanxiao
> > > Sent: Thursday, March 9, 2017 9:28 AM
> > > To: Chris Wilson <chris@chris-wilson.co.uk>;
> > > intel-gfx@lists.freedesktop.org
> > > Cc: intel-gvt-dev@lists.freedesktop.org; Wang, Zhi A
> > > <zhi.a.wang@intel.com>; Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Subject: RE: [PATCH] drm/i915/gvt: Remove bogus retry around
> > > i915_wait_request
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Chris Wilson [mailto:chris.ickle.wilson@gmail.com] On Behalf
> > > > Of Chris Wilson
> > > > Sent: Thursday, March 9, 2017 6:08 AM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: intel-gvt-dev@lists.freedesktop.org; Chris Wilson
> > > > <chris@chris- wilson.co.uk>; Dong, Chuanxiao
> > > > <chuanxiao.dong@intel.com>; Zhenyu
> > > Wang
> > > > <zhenyuw@linux.intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > > > Subject: [PATCH] drm/i915/gvt: Remove bogus retry around
> > > > i915_wait_request
> > > >
> > > > commit 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle
> > > > properly") includes some nonsense to retry a indefinite wait -
> > > > i915_wait_request() does not return until the request is completed
> > > > when used from an uninterruptible context.
> > > >
> > > > Fixes: 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle
> properly"
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > > Cc: Zhi Wang <zhi.a.wang@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/gvt/scheduler.c | 14 +-------------
> > > >  1 file changed, 1 insertion(+), 13 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > > index cfd9f7a38c27..8024904c0d58 100644
> > > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > > @@ -465,19 +465,7 @@ static int workload_thread(void *priv)
> > > >
> > > >  		gvt_dbg_sched("ring id %d wait workload %p\n",
> > > >  				workload->ring_id, workload);
> > > > -retry:
> > > > -		i915_wait_request(workload->req,
> > > > -					 0, MAX_SCHEDULE_TIMEOUT);
> > > > -		/* I915 has replay mechanism and a request will be replayed
> > > > -		 * if there is i915 reset. So the seqno will be updated anyway.
> > > > -		 * If the seqno is not updated yet after waiting, which means
> > > > -		 * the replay may still be in progress and we can wait again.
> > > > -		 */
> > > > -		if (!i915_gem_request_completed(workload->req)) {
> > > > -			gvt_dbg_sched("workload %p not completed, wait
> > > > again\n",
> > > > -					workload);
> > > > -			goto retry;
> > > > -		}
> > > > +		i915_wait_request(workload->req, 0,
> > > > MAX_SCHEDULE_TIMEOUT);
> > >
> > > Thanks Chris for pointing this out. So per the comment,
> > > i915_wait_request will make sure to return until the request
> > > completed, even if a GPU hang happened and replay occurred during
> > > i915_wait_request(), this function will only return after the replay
> completed, right?
> >
> > Hi Chris, ping for the above question. I guess the answer is "yes"?
> 
> Yes. Given MAX_SCHEDULE_TIMEOUT, no INTERRUPTIBLE flags, it will only
> return once the request is completed - that is the hw breadcrumb is advanced
> past the request seqno. That includes waiting across a GPU reset. It is not
> allowed to return another error.

I see. Thanks Chris. My understanding was i915_wait_request() will return without waiting for the replay. Thanks for the fix. :)

Thanks
Chuanxiao

> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request
  2017-03-08 22:08 [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request Chris Wilson
  2017-03-09  0:17 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-03-09  1:28 ` [PATCH] " Dong, Chuanxiao
@ 2017-03-13  9:40 ` Zhenyu Wang
  2 siblings, 0 replies; 7+ messages in thread
From: Zhenyu Wang @ 2017-03-13  9:40 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, intel-gvt-dev


[-- Attachment #1.1: Type: text/plain, Size: 1907 bytes --]

On 2017.03.08 22:08:08 +0000, Chris Wilson wrote:
> commit 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly")
> includes some nonsense to retry a indefinite wait - i915_wait_request()
> does not return until the request is completed when used from an
> uninterruptible context.
> 
> Fixes: 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly"
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chuanxiao Dong <chuanxiao.dong@intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/scheduler.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index cfd9f7a38c27..8024904c0d58 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -465,19 +465,7 @@ static int workload_thread(void *priv)
>  
>  		gvt_dbg_sched("ring id %d wait workload %p\n",
>  				workload->ring_id, workload);
> -retry:
> -		i915_wait_request(workload->req,
> -					 0, MAX_SCHEDULE_TIMEOUT);
> -		/* I915 has replay mechanism and a request will be replayed
> -		 * if there is i915 reset. So the seqno will be updated anyway.
> -		 * If the seqno is not updated yet after waiting, which means
> -		 * the replay may still be in progress and we can wait again.
> -		 */
> -		if (!i915_gem_request_completed(workload->req)) {
> -			gvt_dbg_sched("workload %p not completed, wait again\n",
> -					workload);
> -			goto retry;
> -		}
> +		i915_wait_request(workload->req, 0, MAX_SCHEDULE_TIMEOUT);
>  
>  complete:
>  		gvt_dbg_sched("will complete workload %p, status: %d\n",
> -- 

Applied to gvt-fixes. Thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 163 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-03-13  9:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-08 22:08 [PATCH] drm/i915/gvt: Remove bogus retry around i915_wait_request Chris Wilson
2017-03-09  0:17 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-03-09  1:28 ` [PATCH] " Dong, Chuanxiao
2017-03-13  9:21   ` Dong, Chuanxiao
2017-03-13  9:28     ` Chris Wilson
2017-03-13  9:35       ` Dong, Chuanxiao
2017-03-13  9:40 ` Zhenyu Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.