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 <chris@xxxxxxxxxxxxxxxxxx>
> ---
> 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));
>
> mutex_unlock(&gvt->lock);
Might not need to hold gvt->lock when put request?
> +
> + i915_add_request_no_flush(rq);
Why still add request in error path?
> 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);
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
