Re: [PATCH] media: go7007: return queued buffers on start_streaming() failure

[Hans Verkuil <hverkuil+cisco@kernel.org>]

On 16/10/2026 12:22, YuanZhe Liu wrote:
> The vb2 framework hands ownership of every queued buffer to the driver
> through the buf_queue() callback before start_streaming() is called.  If
> start_streaming() then returns an error, the driver must return those
> buffers to vb2 with vb2_buffer_done() and VB2_BUF_STATE_QUEUED.
> Otherwise vb2_start_streaming() fires WARN_ON(owned_by_drv_count) and
> forcibly reclaims the buffers.
> 
> go7007_start_streaming() returns -EIO when go7007_start_encoder() fails,
> which happens for example when a USB transfer cannot be submitted because
> the device has been disconnected:
> 
>     go7007 1-1:1.0: error submitting audio urb 5: -2
>     go7007 1-1:1.0: error starting stream transfer
>     WARNING: drivers/media/common/videobuf2/videobuf2-core.c:1797 at vb2_start_streaming
> 
> but it leaves the already queued buffers on go->vidq_active.  Give them
> back to vb2 with VB2_BUF_STATE_QUEUED before bailing out, clearing
> go->active_buf as well.
> 
> Fixes: 7955f03d18d1 ("[media] go7007: move out of staging into drivers/media/usb.")
> Cc: stable@vger.kernel.org
> Co-developed-by: Bin Yu <byu@xidian.edu.cn>
> Signed-off-by: Bin Yu <byu@xidian.edu.cn>
> Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
> Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
> Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
> ---
>  drivers/media/usb/go7007/go7007-v4l2.c | 29 +++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/go7007/go7007-v4l2.c b/drivers/media/usb/go7007/go7007-v4l2.c
> index 2087ffcb85a5..e0b646e8dad1 100644
> --- a/drivers/media/usb/go7007/go7007-v4l2.c
> +++ b/drivers/media/usb/go7007/go7007-v4l2.c
> @@ -394,6 +394,31 @@ static void go7007_buf_finish(struct vb2_buffer *vb)
>  	vbuf->field = V4L2_FIELD_NONE;
>  }
>  
> +/*
> + * Return all buffers that have been queued to the driver back to vb2.
> + *
> + * The vb2 framework hands ownership of every queued buffer to the driver
> + * before calling start_streaming().  If start_streaming() fails, those
> + * buffers must be returned with VB2_BUF_STATE_QUEUED, otherwise
> + * vb2_start_streaming() triggers WARN_ON(owned_by_drv_count).
> + */
> +static void go7007_return_queued_buffers(struct go7007 *go)
> +{
> +	struct go7007_buffer *vb, *tmp;
> +	unsigned long flags;
> +	LIST_HEAD(buffers);
> +
> +	spin_lock_irqsave(&go->spinlock, flags);
> +	list_splice_init(&go->vidq_active, &buffers);
> +	go->active_buf = NULL;
> +	spin_unlock_irqrestore(&go->spinlock, flags);

This part should not be needed: if go7007_start_encoder() fails, then active_buf is
still NULL.

> +
> +	list_for_each_entry_safe(vb, tmp, &buffers, list) {
> +		list_del(&vb->list);
> +		vb2_buffer_done(&vb->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
> +	}
> +}
> +
>  static int go7007_start_streaming(struct vb2_queue *q, unsigned int count)
>  {
>  	struct go7007 *go = vb2_get_drv_priv(q);
> @@ -409,8 +434,10 @@ static int go7007_start_streaming(struct vb2_queue *q, unsigned int count)
>  	else
>  		ret = 0;
>  	mutex_unlock(&go->hw_lock);
> -	if (ret)
> +	if (ret) {
> +		go7007_return_queued_buffers(go);

I would just move the code from go7007_return_queued_buffers to here.

Regards,

	Hans

>  		return ret;
> +	}
>  	call_all(&go->v4l2_dev, video, s_stream, 1);
>  	v4l2_ctrl_grab(go->mpeg_video_gop_size, true);
>  	v4l2_ctrl_grab(go->mpeg_video_gop_closure, true);