[Intel-gfx] [PATCH 07/12] drm/i915/gvt: Hold a reference on the request

[Zhenyu Wang]

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 at 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));
>  
>  	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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20161019/850762c8/attachment-0001.sig>