On 2016.10.19 11:11:42 +0100, Chris Wilson wrote:
> The workload took a pointer to the request, and even waited upon,
> without holding a reference on the request. Take that reference
> explicitly and fix up the error path following request allocation that
> missed flushing the request.
>
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/gvt/scheduler.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> b/drivers/gpu/drm/i915/gvt/scheduler.c
> index b15cdf5978a9..224f19ae61ab 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -163,6 +163,7 @@ static int dispatch_workload(struct intel_vgpu_workload
> *workload)
> int ring_id = workload->ring_id;
> struct i915_gem_context *shadow_ctx = workload->vgpu->shadow_ctx;
> struct drm_i915_private *dev_priv = workload->vgpu->gvt->dev_priv;
> + struct drm_i915_gem_request *rq;
> int ret;
>
> gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n",
> @@ -171,17 +172,16 @@ static int dispatch_workload(struct intel_vgpu_workload
> *workload)
> shadow_ctx->desc_template = workload->ctx_desc.addressing_mode <<
> GEN8_CTX_ADDRESSING_MODE_SHIFT;
>
> - workload->req = i915_gem_request_alloc(dev_priv->engine[ring_id],
> - shadow_ctx);
> - if (IS_ERR_OR_NULL(workload->req)) {
> + rq = i915_gem_request_alloc(dev_priv->engine[ring_id], shadow_ctx);
> + if (IS_ERR(rq)) {
> gvt_err("fail to allocate gem request\n");
> - workload->status = PTR_ERR(workload->req);
> - workload->req = NULL;
> + workload->status = PTR_ERR(rq);
> return workload->status;
> }
>
> - gvt_dbg_sched("ring id %d get i915 gem request %p\n",
> - ring_id, workload->req);
> + gvt_dbg_sched("ring id %d get i915 gem request %p\n", ring_id, rq);
> +
> + workload->req = i915_gem_request_get(rq);
>
> mutex_lock(&gvt->lock);
>
> @@ -208,16 +208,16 @@ static int dispatch_workload(struct intel_vgpu_workload
> *workload)
> gvt_dbg_sched("ring id %d submit workload to i915 %p\n",
> ring_id, workload->req);
>
> - i915_add_request_no_flush(workload->req);
> -
> + i915_add_request_no_flush(rq);
> workload->dispatched = true;
> return 0;
> err:
> workload->status = ret;
> - if (workload->req)
> - workload->req = NULL;
> + i915_gem_request_put(fetch_and_zero(&workload->req));
>Looks we don't need put here as in error path from dispatch_workload() we will go with below put path too in main thread. > mutex_unlock(&gvt->lock); > + > + i915_add_request_no_flush(rq); > return ret; > } > > @@ -458,6 +458,8 @@ static int workload_thread(void *priv) > > complete_current_workload(gvt, ring_id); > > + i915_gem_request_put(fetch_and_zero(&workload->req)); > + > if (need_force_wake) > intel_uncore_forcewake_put(gvt->dev_priv, > FORCEWAKE_ALL); -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
signature.asc
Description: PGP signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
