Commit 461fa7b0 authored by Ken Xue's avatar Ken Xue Committed by Alex Deucher

drm/amdgpu: remove ctx->lock

KMD reports a warning on holding a lock from drm_syncobj_find_fence,
when running amdgpu_test case “syncobj timeline test”.

ctx->lock was designed to prevent concurrent "amdgpu_ctx_wait_prev_fence"
calls and avoid dead reservation lock from GPU reset. since no reservation
lock is held in latest GPU reset any more, ctx->lock can be simply removed
and concurrent "amdgpu_ctx_wait_prev_fence" call also can be prevented by
PD root bo reservation lock.

call stacks:
=================
//hold lock
amdgpu_cs_ioctl->amdgpu_cs_parser_init->mutex_lock(&parser->ctx->lock);
…
//report warning
amdgpu_cs_dependencies->amdgpu_cs_process_syncobj_timeline_in_dep \
->amdgpu_syncobj_lookup_and_add_to_sync -> drm_syncobj_find_fence \
-> lockdep_assert_none_held_once
…
amdgpu_cs_ioctl->amdgpu_cs_parser_fini->mutex_unlock(&parser->ctx->lock);
Signed-off-by: default avatarKen Xue <Ken.Xue@amd.com>
Reviewed-by: default avatarChristian König <christian.koenig@amd.com>
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent 8bbd4d83
...@@ -127,8 +127,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs ...@@ -127,8 +127,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
goto free_chunk; goto free_chunk;
} }
mutex_lock(&p->ctx->lock);
/* skip guilty context job */ /* skip guilty context job */
if (atomic_read(&p->ctx->guilty) == 1) { if (atomic_read(&p->ctx->guilty) == 1) {
ret = -ECANCELED; ret = -ECANCELED;
...@@ -585,6 +583,16 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, ...@@ -585,6 +583,16 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
} }
} }
/* Move fence waiting after getting reservation lock of
* PD root. Then there is no need on a ctx mutex lock.
*/
r = amdgpu_ctx_wait_prev_fence(p->ctx, p->entity);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("amdgpu_ctx_wait_prev_fence failed.\n");
goto error_validate;
}
amdgpu_cs_get_threshold_for_moves(p->adev, &p->bytes_moved_threshold, amdgpu_cs_get_threshold_for_moves(p->adev, &p->bytes_moved_threshold,
&p->bytes_moved_vis_threshold); &p->bytes_moved_vis_threshold);
p->bytes_moved = 0; p->bytes_moved = 0;
...@@ -700,7 +708,6 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, ...@@ -700,7 +708,6 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error,
dma_fence_put(parser->fence); dma_fence_put(parser->fence);
if (parser->ctx) { if (parser->ctx) {
mutex_unlock(&parser->ctx->lock);
amdgpu_ctx_put(parser->ctx); amdgpu_ctx_put(parser->ctx);
} }
if (parser->bo_list) if (parser->bo_list)
...@@ -944,7 +951,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev, ...@@ -944,7 +951,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
if (parser->job->uf_addr && ring->funcs->no_user_fence) if (parser->job->uf_addr && ring->funcs->no_user_fence)
return -EINVAL; return -EINVAL;
return amdgpu_ctx_wait_prev_fence(parser->ctx, parser->entity); return 0;
} }
static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p, static int amdgpu_cs_process_fence_dep(struct amdgpu_cs_parser *p,
...@@ -1363,7 +1370,6 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) ...@@ -1363,7 +1370,6 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
goto out; goto out;
r = amdgpu_cs_submit(&parser, cs); r = amdgpu_cs_submit(&parser, cs);
out: out:
amdgpu_cs_parser_fini(&parser, r, reserved_buffers); amdgpu_cs_parser_fini(&parser, r, reserved_buffers);
......
...@@ -230,7 +230,6 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev, ...@@ -230,7 +230,6 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
kref_init(&ctx->refcount); kref_init(&ctx->refcount);
spin_lock_init(&ctx->ring_lock); spin_lock_init(&ctx->ring_lock);
mutex_init(&ctx->lock);
ctx->reset_counter = atomic_read(&adev->gpu_reset_counter); ctx->reset_counter = atomic_read(&adev->gpu_reset_counter);
ctx->reset_counter_query = ctx->reset_counter; ctx->reset_counter_query = ctx->reset_counter;
...@@ -352,7 +351,6 @@ static void amdgpu_ctx_fini(struct kref *ref) ...@@ -352,7 +351,6 @@ static void amdgpu_ctx_fini(struct kref *ref)
} }
} }
amdgpu_ctx_set_stable_pstate(ctx, AMDGPU_CTX_STABLE_PSTATE_NONE); amdgpu_ctx_set_stable_pstate(ctx, AMDGPU_CTX_STABLE_PSTATE_NONE);
mutex_destroy(&ctx->lock);
kfree(ctx); kfree(ctx);
} }
......
...@@ -49,7 +49,6 @@ struct amdgpu_ctx { ...@@ -49,7 +49,6 @@ struct amdgpu_ctx {
bool preamble_presented; bool preamble_presented;
int32_t init_priority; int32_t init_priority;
int32_t override_priority; int32_t override_priority;
struct mutex lock;
atomic_t guilty; atomic_t guilty;
unsigned long ras_counter_ce; unsigned long ras_counter_ce;
unsigned long ras_counter_ue; unsigned long ras_counter_ue;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment