drm/i915/gvt: Fix workload status after wait
diff mbox

Message ID 20161102054137.25612-1-zhenyuw@linux.intel.com
State New
Headers show

Commit Message

Zhenyu Wang Nov. 2, 2016, 5:41 a.m. UTC
From commit e95433c73a11759203af1cae5958f998c9673370, workload status setting
was changed to only capture on error path, but we need to set it properly in
normal path too, otherwise we'll fail to complete workload which could lead
guest VM vGPU reset.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/gvt/scheduler.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Joonas Lahtinen Nov. 2, 2016, 6:47 a.m. UTC | #1
On ke, 2016-11-02 at 13:41 +0800, Zhenyu Wang wrote:
> From commit e95433c73a11759203af1cae5958f998c9673370, workload status setting
> was changed to only capture on error path, but we need to set it properly in
> normal path too, otherwise we'll fail to complete workload which could lead
> guest VM vGPU reset.
> 

Should have Fixes tag with the above commit.

> @@ -455,7 +455,8 @@ static int workload_thread(void *priv)
>  		if (lret < 0) {
>  			workload->status = lret;
>  			gvt_err("fail to wait workload, skip\n");
> -		}
> +		} else
> +			workload->status = 0;

All branches of if-else continuum must use braces if one does, so
"} else {" here

Regards, Joonas
Zhenyu Wang Nov. 2, 2016, 7:07 a.m. UTC | #2
On 2016.11.02 08:47:52 +0200, Joonas Lahtinen wrote:
> On ke, 2016-11-02 at 13:41 +0800, Zhenyu Wang wrote:
> > From commit e95433c73a11759203af1cae5958f998c9673370, workload status setting
> > was changed to only capture on error path, but we need to set it properly in
> > normal path too, otherwise we'll fail to complete workload which could lead
> > guest VM vGPU reset.
> > 
> 
> Should have Fixes tag with the above commit.
> 
> > @@ -455,7 +455,8 @@ static int workload_thread(void *priv)
> >  		if (lret < 0) {
> >  			workload->status = lret;
> >  			gvt_err("fail to wait workload, skip\n");
> > -		}
> > +		} else
> > +			workload->status = 0;
> 
> All branches of if-else continuum must use braces if one does, so
> "} else {" here
> 

Thanks for the review. I'll queue this up in gvt-linux tree as it stopped
our testing and will be included in next pull request.
Saarinen, Jani Nov. 2, 2016, 7:08 a.m. UTC | #3
> == Series Details ==

> 

> Series: drm/i915/gvt: Fix workload status after wait

> URL   : https://patchwork.freedesktop.org/series/14711/

> State : failure

> 

> == Summary ==

> 

> Series 14711v1 drm/i915/gvt: Fix workload status after wait

> https://patchwork.freedesktop.org/api/1.0/series/14711/revisions/1/mbox/

> 

> Test drv_module_reload_basic:

>                 pass       -> DMESG-WARN (fi-ilk-650)

> Test gem_ctx_switch:

>         Subgroup basic-default:

>                 pass       -> TIMEOUT    (fi-bsw-n3050)

Seen also on CI: https://intel-gfx-ci.01.org/CI/fi-bsw-n3050.html => (CI_DRM_1772 / Oct 27).

running: igt/gem_ctx_switch/basic-default
[021/241] pass: 21 /                     
timeout: igt/gem_ctx_switch/basic-default
[022/241] pass: 21, timeout: 1 /
running: igt/gem_ctx_switch/basic-default-heavy
[022/241] pass: 21, timeout: 1 -               
Build timed out (after 17 minutes). Marking the build as aborted.

>         Subgroup basic-default-heavy:

>                 pass       -> INCOMPLETE (fi-bsw-n3050)

> Test gem_exec_suspend:

>         Subgroup basic-s3:

>                 pass       -> DMESG-WARN (fi-ilk-650)

> Test kms_busy:

>         Subgroup basic-flip-default-a:

>                 pass       -> DMESG-WARN (fi-ilk-650)

> Test kms_pipe_crc_basic:

>         Subgroup bad-nb-words-1:

>                 pass       -> DMESG-WARN (fi-ilk-650)

These all ILK-650 cases seems to be https://bugs.freedesktop.org/show_bug.cgi?id=98531 ?

>         Subgroup bad-source:

>                 dmesg-warn -> PASS       (fi-ilk-650)

>         Subgroup hang-read-crc-pipe-a:

>                 dmesg-warn -> PASS       (fi-ilk-650)

>         Subgroup nonblocking-crc-pipe-a-frame-sequence:

>                 dmesg-warn -> PASS       (fi-ilk-650)

>         Subgroup read-crc-pipe-a:

>                 pass       -> DMESG-WARN (fi-ilk-650)

>         Subgroup suspend-read-crc-pipe-a:

>                 dmesg-warn -> PASS       (fi-ilk-650)

> 

> fi-bsw-n3050     total:23   pass:21   dwarn:0   dfail:0   fail:0   skip:0

> fi-ilk-650       total:241  pass:181  dwarn:6   dfail:0   fail:0   skip:54

> 

> c5ad9c11e819eebcad5b9be5aa5e991e89b26965 drm-intel-nightly: 2016y-

> 11m-01d-16h-36m-25s UTC integration manifest

> 6995b34 drm/i915/gvt: Fix workload status after wait

> 

> == Logs ==

> 

> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2883/



Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 18acb45..bc4c14a 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -455,7 +455,8 @@  static int workload_thread(void *priv)
 		if (lret < 0) {
 			workload->status = lret;
 			gvt_err("fail to wait workload, skip\n");
-		}
+		} else
+			workload->status = 0;
 
 complete:
 		gvt_dbg_sched("will complete workload %p\n, status: %d\n",