[PATCH 1/1] drm/qxl: add mutex_lock/mutex_unlock to ensure the order in which resources are released.
From: Caicai
Date: Sat Apr 18 2020 - 02:41:29 EST
When a qxl resource is released, the list that needs to be released is
fetched from the linked list ring and cleared. When you empty the list,
instead of trying to determine whether the ttm buffer object for each
qxl in the list is locked, you release the qxl object and remove the
element from the list until the list is empty. It was found that the
linked list was cleared first, and that the lock on the corresponding
ttm Bo for the QXL had not been released, so that the new qxl could not
be locked when it used the TTM. By adding an additional mutex to ensure
timing, qxl elements are not allowed to be removed from the list until
ttm Bo's lock is released
Signed-off-by: Caicai <caizhaopeng@xxxxxxxxxxxxx>
---
drivers/gpu/drm/qxl/qxl_cmd.c | 12 +++++++++++-
drivers/gpu/drm/qxl/qxl_display.c | 15 +++++++++++++++
drivers/gpu/drm/qxl/qxl_draw.c | 19 +++++++++++++++++++
drivers/gpu/drm/qxl/qxl_drv.h | 3 ++-
drivers/gpu/drm/qxl/qxl_ioctl.c | 3 +++
drivers/gpu/drm/qxl/qxl_release.c | 2 ++
6 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 208af9f37914..484405dae253 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -203,7 +203,9 @@ bool qxl_queue_garbage_collect(struct qxl_device *qdev, bool flush)
if (!qxl_check_idle(qdev->release_ring)) {
schedule_work(&qdev->gc_work);
if (flush)
- flush_work(&qdev->gc_work);
+ //can't flush work, it may lead to deadlock
+ usleep_range(500, 1000);
+
return true;
}
return false;
@@ -241,7 +243,11 @@ int qxl_garbage_collect(struct qxl_device *qdev)
}
id = next_id;
+ mutex_lock(&release->async_release_mutex);
+
qxl_release_free(qdev, release);
+
+ mutex_unlock(&release->async_release_mutex);
++i;
}
}
@@ -475,6 +481,8 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,
if (ret)
return ret;
+ mutex_lock(&release->async_release_mutex);
+
cmd = (struct qxl_surface_cmd *)qxl_release_map(qdev, release);
cmd->type = QXL_SURFACE_CMD_CREATE;
cmd->flags = QXL_SURF_FLAG_KEEP_DATA;
@@ -503,6 +511,8 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,
qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false);
qxl_release_fence_buffer_objects(release);
+ mutex_unlock(&release->async_release_mutex);
+
surf->hw_surf_alloc = true;
spin_lock(&qdev->surf_id_idr_lock);
idr_replace(&qdev->surf_id_idr, surf, surf->surface_id);
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 0570c6826bff..04bfb181a402 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -522,6 +522,8 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane)
if (ret)
goto out_free_release;
+ mutex_lock(&release->async_release_mutex);
+
cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev, release);
cmd->type = QXL_CURSOR_SET;
cmd->u.set.position.x = plane->state->crtc_x + fb->hot_x;
@@ -535,6 +537,8 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane)
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_fence_buffer_objects(release);
+ mutex_unlock(&release->async_release_mutex);
+
return ret;
out_free_release:
@@ -653,6 +657,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
if (ret)
goto out_free_bo;
+ mutex_lock(&release->async_release_mutex);
+
ret = qxl_bo_kmap(cursor_bo, (void **)&cursor);
if (ret)
goto out_backoff;
@@ -686,6 +692,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
if (ret)
goto out_free_release;
+ mutex_lock(&release->async_release_mutex);
+
cmd = (struct qxl_cursor_cmd *) qxl_release_map(qdev, release);
cmd->type = QXL_CURSOR_MOVE;
}
@@ -697,6 +705,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_fence_buffer_objects(release);
+ mutex_unlock(&release->async_release_mutex);
+
if (old_cursor_bo)
qxl_bo_unref(&old_cursor_bo);
@@ -706,6 +716,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
out_backoff:
qxl_release_backoff_reserve_list(release);
+ mutex_unlock(&release->async_release_mutex);
out_free_bo:
qxl_bo_unref(&cursor_bo);
out_kunmap:
@@ -736,12 +747,16 @@ static void qxl_cursor_atomic_disable(struct drm_plane *plane,
return;
}
+ mutex_lock(&release->async_release_mutex);
+
cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev, release);
cmd->type = QXL_CURSOR_HIDE;
qxl_release_unmap(qdev, release, &cmd->release_info);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_fence_buffer_objects(release);
+
+ mutex_unlock(&release->async_release_mutex);
}
static int qxl_plane_prepare_fb(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c
index 4d8681e84e68..4dc24739c79c 100644
--- a/drivers/gpu/drm/qxl/qxl_draw.c
+++ b/drivers/gpu/drm/qxl/qxl_draw.c
@@ -192,6 +192,8 @@ void qxl_draw_opaque_fb(const struct qxl_fb_image *qxl_fb_image,
if (ret)
goto out_free_palette;
+ mutex_lock(&release->async_release_mutex);
+
rect.left = x;
rect.right = x + width;
rect.top = y;
@@ -200,6 +202,7 @@ void qxl_draw_opaque_fb(const struct qxl_fb_image *qxl_fb_image,
ret = make_drawable(qdev, 0, QXL_DRAW_COPY, &rect, release);
if (ret) {
qxl_release_backoff_reserve_list(release);
+ mutex_unlock(&release->async_release_mutex);
goto out_free_palette;
}
@@ -208,6 +211,7 @@ void qxl_draw_opaque_fb(const struct qxl_fb_image *qxl_fb_image,
width, height, depth, stride);
if (ret) {
qxl_release_backoff_reserve_list(release);
+ mutex_unlock(&release->async_release_mutex);
qxl_release_free(qdev, release);
return;
}
@@ -244,6 +248,8 @@ void qxl_draw_opaque_fb(const struct qxl_fb_image *qxl_fb_image,
qxl_push_command_ring_release(qdev, release, QXL_CMD_DRAW, false);
qxl_release_fence_buffer_objects(release);
+ mutex_unlock(&release->async_release_mutex);
+
out_free_palette:
if (palette_bo)
qxl_bo_unref(&palette_bo);
@@ -326,6 +332,8 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
if (ret)
goto out_free_image;
+ mutex_lock(&release->async_release_mutex);
+
drawable_rect.left = left;
drawable_rect.right = right;
drawable_rect.top = top;
@@ -387,6 +395,7 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev,
out_release_backoff:
if (ret)
qxl_release_backoff_reserve_list(release);
+ mutex_unlock(&release->async_release_mutex);
out_free_image:
qxl_image_free_objects(qdev, dimage);
out_free_clips:
@@ -417,6 +426,8 @@ void qxl_draw_copyarea(struct qxl_device *qdev,
if (ret)
goto out_free_release;
+ mutex_lock(&release->async_release_mutex);
+
rect.left = dx;
rect.top = dy;
rect.right = dx + width;
@@ -424,6 +435,7 @@ void qxl_draw_copyarea(struct qxl_device *qdev,
ret = make_drawable(qdev, 0, QXL_COPY_BITS, &rect, release);
if (ret) {
qxl_release_backoff_reserve_list(release);
+ mutex_unlock(&release->async_release_mutex);
goto out_free_release;
}
@@ -435,6 +447,8 @@ void qxl_draw_copyarea(struct qxl_device *qdev,
qxl_push_command_ring_release(qdev, release, QXL_CMD_DRAW, false);
qxl_release_fence_buffer_objects(release);
+ mutex_unlock(&release->async_release_mutex);
+
out_free_release:
if (ret)
free_drawable(qdev, release);
@@ -459,9 +473,12 @@ void qxl_draw_fill(struct qxl_draw_fill *qxl_draw_fill_rec)
if (ret)
goto out_free_release;
+ mutex_lock(&release->async_release_mutex);
+
ret = make_drawable(qdev, 0, QXL_DRAW_FILL, &rect, release);
if (ret) {
qxl_release_backoff_reserve_list(release);
+ mutex_unlock(&release->async_release_mutex);
goto out_free_release;
}
@@ -479,6 +496,8 @@ void qxl_draw_fill(struct qxl_draw_fill *qxl_draw_fill_rec)
qxl_push_command_ring_release(qdev, release, QXL_CMD_DRAW, false);
qxl_release_fence_buffer_objects(release);
+ mutex_unlock(&release->async_release_mutex);
+
out_free_release:
if (ret)
free_drawable(qdev, release);
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 01220d386b0a..cbc7bdb5b8d3 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -54,7 +54,7 @@
#define DRIVER_NAME "qxl"
#define DRIVER_DESC "RH QXL"
-#define DRIVER_DATE "20120117"
+#define DRIVER_DATE "20200107"
#define DRIVER_MAJOR 0
#define DRIVER_MINOR 1
@@ -172,6 +172,7 @@ struct qxl_release {
uint32_t surface_release_id;
struct ww_acquire_ctx ticket;
struct list_head bos;
+ struct mutex async_release_mutex;
};
struct qxl_drm_chunk {
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 6cc9f3367fa0..189d016b871b 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -250,6 +250,8 @@ static int qxl_process_single_command(struct qxl_device *qdev,
if (ret)
goto out_free_bos;
+ mutex_lock(&release->async_release_mutex);
+
for (i = 0; i < cmd->relocs_num; ++i) {
if (reloc_info[i].type == QXL_RELOC_TYPE_BO)
apply_reloc(qdev, &reloc_info[i]);
@@ -263,6 +265,7 @@ static int qxl_process_single_command(struct qxl_device *qdev,
else
qxl_release_fence_buffer_objects(release);
+ mutex_unlock(&release->async_release_mutex);
out_free_bos:
out_free_release:
if (ret)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index e37f0097f744..f4c066dcabfe 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -133,6 +133,7 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
release->type = type;
release->release_offset = 0;
release->surface_release_id = 0;
+ mutex_init(&release->async_release_mutex);
INIT_LIST_HEAD(&release->bos);
idr_preload(GFP_KERNEL);
@@ -343,6 +344,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev, unsigned long size,
}
mutex_lock(&qdev->release_mutex);
+
if (qdev->current_release_bo_offset[cur_idx] + 1 >= releases_per_bo[cur_idx]) {
qxl_bo_unref(&qdev->current_release_bo[cur_idx]);
qdev->current_release_bo_offset[cur_idx] = 0;
--
2.20.1