Browse Source

drm/amdgpu: make the CTX ioctl thread-safe

The existing locks were protecting the list, but not the elements.

v2: rename hlock to lock

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com>
Marek Olšák 10 years ago
parent
commit
0147ee0f59

+ 1 - 1
drivers/gpu/drm/amd/amdgpu/amdgpu.h

@@ -1056,7 +1056,7 @@ struct amdgpu_ctx_mgr {
 	struct amdgpu_device *adev;
 	struct amdgpu_device *adev;
 	struct idr ctx_handles;
 	struct idr ctx_handles;
 	/* lock for IDR system */
 	/* lock for IDR system */
-	struct mutex hlock;
+	struct mutex lock;
 };
 };
 
 
 /*
 /*

+ 10 - 10
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c

@@ -33,9 +33,7 @@ static void amdgpu_ctx_do_release(struct kref *ref)
 	ctx = container_of(ref, struct amdgpu_ctx, refcount);
 	ctx = container_of(ref, struct amdgpu_ctx, refcount);
 	mgr = &ctx->fpriv->ctx_mgr;
 	mgr = &ctx->fpriv->ctx_mgr;
 
 
-	mutex_lock(&mgr->hlock);
 	idr_remove(&mgr->ctx_handles, ctx->id);
 	idr_remove(&mgr->ctx_handles, ctx->id);
-	mutex_unlock(&mgr->hlock);
 	kfree(ctx);
 	kfree(ctx);
 }
 }
 
 
@@ -49,20 +47,20 @@ int amdgpu_ctx_alloc(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv, uin
 	if (!ctx)
 	if (!ctx)
 		return -ENOMEM;
 		return -ENOMEM;
 
 
-	mutex_lock(&mgr->hlock);
+	mutex_lock(&mgr->lock);
 	r = idr_alloc(&mgr->ctx_handles, ctx, 0, 0, GFP_KERNEL);
 	r = idr_alloc(&mgr->ctx_handles, ctx, 0, 0, GFP_KERNEL);
 	if (r < 0) {
 	if (r < 0) {
-		mutex_unlock(&mgr->hlock);
+		mutex_unlock(&mgr->lock);
 		kfree(ctx);
 		kfree(ctx);
 		return r;
 		return r;
 	}
 	}
-	mutex_unlock(&mgr->hlock);
 	*id = (uint32_t)r;
 	*id = (uint32_t)r;
 
 
 	memset(ctx, 0, sizeof(*ctx));
 	memset(ctx, 0, sizeof(*ctx));
 	ctx->id = *id;
 	ctx->id = *id;
 	ctx->fpriv = fpriv;
 	ctx->fpriv = fpriv;
 	kref_init(&ctx->refcount);
 	kref_init(&ctx->refcount);
+	mutex_unlock(&mgr->lock);
 
 
 	return 0;
 	return 0;
 }
 }
@@ -72,13 +70,14 @@ int amdgpu_ctx_free(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv, uint
 	struct amdgpu_ctx *ctx;
 	struct amdgpu_ctx *ctx;
 	struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
 	struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
 
 
-	rcu_read_lock();
+	mutex_lock(&mgr->lock);
 	ctx = idr_find(&mgr->ctx_handles, id);
 	ctx = idr_find(&mgr->ctx_handles, id);
-	rcu_read_unlock();
 	if (ctx) {
 	if (ctx) {
 		kref_put(&ctx->refcount, amdgpu_ctx_do_release);
 		kref_put(&ctx->refcount, amdgpu_ctx_do_release);
+		mutex_unlock(&mgr->lock);
 		return 0;
 		return 0;
 	}
 	}
+	mutex_unlock(&mgr->lock);
 	return -EINVAL;
 	return -EINVAL;
 }
 }
 
 
@@ -87,14 +86,15 @@ int amdgpu_ctx_query(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv, uin
 	struct amdgpu_ctx *ctx;
 	struct amdgpu_ctx *ctx;
 	struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
 	struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
 
 
-	rcu_read_lock();
+	mutex_lock(&mgr->lock);
 	ctx = idr_find(&mgr->ctx_handles, id);
 	ctx = idr_find(&mgr->ctx_handles, id);
-	rcu_read_unlock();
 	if (ctx) {
 	if (ctx) {
 		/* state should alter with CS activity */
 		/* state should alter with CS activity */
 		*state = ctx->state;
 		*state = ctx->state;
+		mutex_unlock(&mgr->lock);
 		return 0;
 		return 0;
 	}
 	}
+	mutex_unlock(&mgr->lock);
 	return -EINVAL;
 	return -EINVAL;
 }
 }
 
 
@@ -111,7 +111,7 @@ void amdgpu_ctx_fini(struct amdgpu_fpriv *fpriv)
 			DRM_ERROR("ctx (id=%ul) is still alive\n",ctx->id);
 			DRM_ERROR("ctx (id=%ul) is still alive\n",ctx->id);
 	}
 	}
 
 
-	mutex_destroy(&mgr->hlock);
+	mutex_destroy(&mgr->lock);
 }
 }
 
 
 int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,

+ 1 - 1
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

@@ -497,7 +497,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 	idr_init(&fpriv->bo_list_handles);
 	idr_init(&fpriv->bo_list_handles);
 
 
 	/* init context manager */
 	/* init context manager */
-	mutex_init(&fpriv->ctx_mgr.hlock);
+	mutex_init(&fpriv->ctx_mgr.lock);
 	idr_init(&fpriv->ctx_mgr.ctx_handles);
 	idr_init(&fpriv->ctx_mgr.ctx_handles);
 	fpriv->ctx_mgr.adev = adev;
 	fpriv->ctx_mgr.adev = adev;