[PATCH v2 1/9] media: chips-media: wave5: Ensure Atomic Access to src_buf list

[Jackson.lee]

From: Brandon Brnich <b-brnich@xxxxxx>

feed_lock was introduced in decoder performance improvements. This was
used to ensure atomic access to the driver's avail_src_buf list during
fill_ringbuffer, streamoff_output, and buf_queue. While this protected
the drivers available src bufs, the v4l2_m2m_src_bufs were not being
protected at driver level. Three separate threads have access to remove
buffers: start_decode fail path, streamoff_output, and finish_decode.

In streamoff_output(), replace the inst_src_buf_remove() loop with
list_for_each_entry_safe(). inst_src_buf_remove() takes feed_lock
itself, so calling it while streamoff_output() holds feed_lock would
take the same mutex twice and deadlock. Unlink each entry with
list_del_init() directly instead.

In wave5_handle_src_buffer(), feed_lock does not serialize against
v4l2_m2m_buf_queue(), which buf_queue calls after dropping feed_lock and
which adds to the ready queue under rdy_spinlock only. Walking the ready
queue with v4l2_m2m_for_each_src_buf_safe() therefore races with
VIDIOC_QBUF. The loop only ever looks at the head of the queue, so peek
it with v4l2_m2m_next_src_buf(), which takes rdy_spinlock, instead.

Fixes: a176ac5e701f ("media: chips-media: wave5: Improve performance of decoder")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Brandon Brnich <b-brnich@xxxxxx>
Signed-off-by: Jackson Lee <jackson.lee@xxxxxxxxxxxxxxx>
Signed-off-by: Nas Chung <nas.chung@xxxxxxxxxxxxxxx>
---
 .../platform/chips-media/wave5/wave5-vpu-dec.c  | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 6564cf3ec739..5993102e5afe 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -180,7 +180,7 @@ static bool wave5_last_src_buffer_consumed(struct v4l2_m2m_ctx *m2m_ctx)
 static void wave5_handle_src_buffer(struct vpu_instance *inst, dma_addr_t rd_ptr)
 {
 	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
-	struct v4l2_m2m_buffer *buf, *n;
+	struct vb2_v4l2_buffer *src_buf;
 	size_t consumed_bytes = 0;
 
 	if (rd_ptr >= inst->last_rd_ptr) {
@@ -198,8 +198,8 @@ static void wave5_handle_src_buffer(struct vpu_instance *inst, dma_addr_t rd_ptr
 	dev_dbg(inst->dev->dev, "%s: %zu bytes of bitstream was consumed", __func__,
 		consumed_bytes);
 
-	v4l2_m2m_for_each_src_buf_safe(m2m_ctx, buf, n) {
-		struct vb2_v4l2_buffer *src_buf = &buf->vb;
+	mutex_lock(&inst->feed_lock);
+	while ((src_buf = v4l2_m2m_next_src_buf(m2m_ctx))) {
 		size_t src_size = vb2_get_plane_payload(&src_buf->vb2_buf, 0);
 
 		if (src_size > consumed_bytes)
@@ -224,6 +224,7 @@ static void wave5_handle_src_buffer(struct vpu_instance *inst, dma_addr_t rd_ptr
 			break;
 		}
 	}
+	mutex_unlock(&inst->feed_lock);
 
 	inst->remaining_consumed_bytes = consumed_bytes;
 }
@@ -237,9 +238,11 @@ static int start_decode(struct vpu_instance *inst, u32 *fail_res)
 	if (ret) {
 		struct vb2_v4l2_buffer *src_buf;
 
+		mutex_lock(&inst->feed_lock);
 		src_buf = v4l2_m2m_src_buf_remove(m2m_ctx);
 		if (src_buf)
 			v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_ERROR);
+		mutex_unlock(&inst->feed_lock);
 		set_instance_state(inst, VPU_INST_STATE_STOP);
 
 		dev_dbg(inst->dev->dev, "%s: pic run failed / finish job", __func__);
@@ -1453,12 +1456,13 @@ static int streamoff_output(struct vb2_queue *q)
 	dma_addr_t new_rd_ptr;
 	struct dec_output_info dec_info;
 	unsigned int i;
-	struct vpu_src_buffer *vpu_buf;
+	struct vpu_src_buffer *vpu_buf, *tmp;
 
 	inst->retry = false;
 	inst->queuing_num = 0;
-	while ((vpu_buf = inst_src_buf_remove(inst)) != NULL)
-		;
+	mutex_lock(&inst->feed_lock);
+	list_for_each_entry_safe(vpu_buf, tmp, &inst->avail_src_bufs, list)
+		list_del_init(&vpu_buf->list);
 
 	for (i = 0; i < v4l2_m2m_num_dst_bufs_ready(m2m_ctx); i++) {
 		ret = wave5_vpu_dec_set_disp_flag(inst, i);
@@ -1473,6 +1477,7 @@ static int streamoff_output(struct vb2_queue *q)
 			__func__, buf->vb2_buf.type, buf->vb2_buf.index);
 		v4l2_m2m_buf_done(buf, VB2_BUF_STATE_ERROR);
 	}
+	mutex_unlock(&inst->feed_lock);
 
 	while (wave5_vpu_dec_get_output_info(inst, &dec_info) == 0) {
 		if (dec_info.index_frame_display >= 0)

base-commit: 58348f64125e9a3e44d3abb275ca7f4e6c9641e5
-- 
2.43.0