[PATCH v2] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS

[PATCH v2] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS

[Karl Mehltretter]

VIDIOC_PREPARE_BUF leaves a buffer DEQUEUED after calling buf_prepare()
and the prepare() memory operation. VIDIOC_REMOVE_BUFS accepts that state
but frees the buffer without calling the matching finish operations.

With VIDEO_ADV_DEBUG, vivid reports:

  unbalanced counters for queue 000000009fa716f8, buffer 0:
       buf_prepare: 1 buf_finish: 0
       prepare: 1 finish: 0

Factor the per-buffer cancellation tail out of __vb2_queue_cancel() into
__vb2_buf_cancel() and call it from vb2_core_remove_bufs() after the
range check, so prepared buffers are finished and reinitialized before
being freed.

Fixes: a3293a85381e ("media: v4l2: Add REMOVE_BUFS ioctl")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
v2:
  - Factor the per-buffer cancellation tail out of __vb2_queue_cancel()
    into __vb2_buf_cancel() and call it from vb2_core_remove_bufs()
    after the range check (Tomasz Figa).
  - Send standalone ahead of the rest of the series while the cache-API
    questions on patches 2-4 are resolved:
    https://lore.kernel.org/r/20261019063401.3674-2-kmehltretter@gmail.com/

 .../media/common/videobuf2/videobuf2-core.c   | 88 ++++++++++---------
 1 file changed, 47 insertions(+), 41 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index b0a6084f17573..29e539ee002e9 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1717,6 +1717,46 @@ int vb2_core_prepare_buf(struct vb2_queue *q, struct vb2_buffer *vb, void *pb)
 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+static void __vb2_buf_cancel(struct vb2_buffer *vb)
+{
+	struct media_request *req = vb->req_obj.req;
+
+	/*
+	 * If a request is associated with this buffer, then
+	 * call buf_request_cancel() to give the driver to complete()
+	 * related request objects. Otherwise those objects would
+	 * never complete.
+	 */
+	if (req) {
+		enum media_request_state state;
+		unsigned long flags;
+
+		spin_lock_irqsave(&req->lock, flags);
+		state = req->state;
+		spin_unlock_irqrestore(&req->lock, flags);
+
+		if (state == MEDIA_REQUEST_STATE_QUEUED)
+			call_void_vb_qop(vb, buf_request_complete, vb);
+	}
+
+	__vb2_buf_mem_finish(vb);
+
+	if (vb->prepared) {
+		call_void_vb_qop(vb, buf_finish, vb);
+		vb->prepared = 0;
+	}
+	__vb2_dqbuf(vb);
+
+	if (vb->req_obj.req) {
+		media_request_object_unbind(&vb->req_obj);
+		media_request_object_put(&vb->req_obj);
+	}
+	if (vb->request)
+		media_request_put(vb->request);
+	vb->request = NULL;
+	vb->copied_timestamp = 0;
+}
+
 int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int count)
 {
 	unsigned int i, ret = 0;
@@ -1746,6 +1786,10 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
 			goto unlock;
 		}
 	}
+
+	for (i = start; i < start + count; i++)
+		__vb2_buf_cancel(vb2_get_buffer(q, i));
+
 	__vb2_queue_free(q, start, count);
 	dprintk(q, 2, "%u buffers removed\n", count);
 
@@ -2253,48 +2297,10 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 	 * be changed, so we can't move the buf_finish() to __vb2_dqbuf().
 	 */
 	for (i = 0; i < q->max_num_buffers; i++) {
-		struct vb2_buffer *vb;
-		struct media_request *req;
-
-		vb = vb2_get_buffer(q, i);
-		if (!vb)
-			continue;
-
-		req = vb->req_obj.req;
-		/*
-		 * If a request is associated with this buffer, then
-		 * call buf_request_cancel() to give the driver to complete()
-		 * related request objects. Otherwise those objects would
-		 * never complete.
-		 */
-		if (req) {
-			enum media_request_state state;
-			unsigned long flags;
-
-			spin_lock_irqsave(&req->lock, flags);
-			state = req->state;
-			spin_unlock_irqrestore(&req->lock, flags);
-
-			if (state == MEDIA_REQUEST_STATE_QUEUED)
-				call_void_vb_qop(vb, buf_request_complete, vb);
-		}
-
-		__vb2_buf_mem_finish(vb);
-
-		if (vb->prepared) {
-			call_void_vb_qop(vb, buf_finish, vb);
-			vb->prepared = 0;
-		}
-		__vb2_dqbuf(vb);
+		struct vb2_buffer *vb = vb2_get_buffer(q, i);
 
-		if (vb->req_obj.req) {
-			media_request_object_unbind(&vb->req_obj);
-			media_request_object_put(&vb->req_obj);
-		}
-		if (vb->request)
-			media_request_put(vb->request);
-		vb->request = NULL;
-		vb->copied_timestamp = 0;
+		if (vb)
+			__vb2_buf_cancel(vb);
 	}
 }
 

base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.39.5 (Apple Git-154)