Re: [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS

[Tomasz Figa]

Hi Karl,

On Mon, Oct 19, 2026 at 4:14 PM Karl Mehltretter <kmehltretter@xxxxxxxxx> wrote:
>
> 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
>
> Finish prepared buffers before freeing them, as queue cancellation does.
>
> Fixes: a3293a85381e ("media: v4l2: Add REMOVE_BUFS ioctl")
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Thanks for the patch. Please take a look at my comments inline.

>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index b0a6084f17573..460eaf4e82f78 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -1746,6 +1746,19 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
>                         goto unlock;
>                 }
>         }
> +
> +       /* A buffer can be prepared without being queued. Finish it first. */
> +       for (i = start; i < start + count; i++) {
> +               struct vb2_buffer *vb = vb2_get_buffer(q, i);
> +
> +               if (!vb->prepared)
> +                       continue;
> +
> +               __vb2_buf_mem_finish(vb);
> +               call_void_vb_qop(vb, buf_finish, vb);
> +               vb->prepared = 0;
> +       }
> +

__vb2_queue_cancel() does much more than that, especially all the
media request handling. In addition, this open coded handling just
causes code duplication.

Would it make more sense to refactor the relevant part of the buffer
cancelation loop from __vb2_queue_cancel() into a helper function and
call it here instead?

Best,
Tomasz

>         __vb2_queue_free(q, start, count);
>         dprintk(q, 2, "%u buffers removed\n", count);
>
> --
> 2.47.3
>