On 2016.11.24 01:17:06 +0300, Dan Carpenter wrote:
> Hello Pei Zhang,
>
> The patch 90d27a1b180e: "drm/i915/gvt: fix deadlock in
> workload_thread" from Nov 14, 2016, leads to the following static
> checker warning:
>
> drivers/gpu/drm/i915/gvt/scheduler.c:217 dispatch_workload()
> warn: inconsistent returns 'mutex:&dev_priv->drm.struct_mutex'.
>
> drivers/gpu/drm/i915/gvt/scheduler.c
> 161 static int dispatch_workload(struct intel_vgpu_workload *workload)
> 162 {
> 163 int ring_id = workload->ring_id;
> 164 struct i915_gem_context *shadow_ctx =
> workload->vgpu->shadow_ctx;
> 165 struct drm_i915_private *dev_priv =
> workload->vgpu->gvt->dev_priv;
> 166 struct drm_i915_gem_request *rq;
> 167 int ret;
> 168
> 169 gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n",
> 170 ring_id, workload);
> 171
> 172 shadow_ctx->desc_template =
> workload->ctx_desc.addressing_mode <<
> 173 GEN8_CTX_ADDRESSING_MODE_SHIFT;
> 174
> 175 mutex_lock(&dev_priv->drm.struct_mutex);
> 176
> 177 rq = i915_gem_request_alloc(dev_priv->engine[ring_id],
> shadow_ctx);
> 178 if (IS_ERR(rq)) {
> 179 gvt_err("fail to allocate gem request\n");
> 180 workload->status = PTR_ERR(rq);
> 181 return workload->status;
>
> We're holding the lock here, which is obviously a bug. But also should
> we goto out? I always thought that functions with an "out" label were
> future proof?
>
Thanks, Dan. Yes, missed alloc failure path. How about below one? Pei, is it
fine for you?
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
b/drivers/gpu/drm/i915/gvt/scheduler.c
index f898df3..4db2422 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -177,8 +177,8 @@ static int dispatch_workload(struct intel_vgpu_workload
*workload)
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(rq);
- return workload->status;
+ ret = PTR_ERR(rq);
+ goto out;
}
gvt_dbg_sched("ring id %d get i915 gem request %p\n", ring_id, rq);
@@ -212,7 +212,8 @@ static int dispatch_workload(struct intel_vgpu_workload
*workload)
if (ret)
workload->status = ret;
- i915_add_request_no_flush(rq);
+ if (!IS_ERR_OR_NULL(rq))
+ i915_add_request_no_flush(rq);
mutex_unlock(&dev_priv->drm.struct_mutex);
return ret;
}
@@ -460,7 +461,8 @@ static int workload_thread(void *priv)
complete_current_workload(gvt, ring_id);
- i915_gem_request_put(fetch_and_zero(&workload->req));
+ if (workload->req)
+ i915_gem_request_put(fetch_and_zero(&workload->req));
if (need_force_wake)
intel_uncore_forcewake_put(gvt->dev_priv,
--
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
