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

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

[YuanZhe Liu]

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);
+
+	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);
 		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);
-- 
2.34.1

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

[YuanZhe Liu]

Hi Hans and Mauro,

Could you please review this patch?

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

It returns queued go7007 buffers in the QUEUED state when
start_streaming() fails.

The patch is unchanged. I am copying these addresses for visibility.

Original submission:
https://lore.kernel.org/r/20261016102213.42771-1-25031212351@stu.xidian.edu.cn

Thanks,
YuanZhe

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

[Hans Verkuil]

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);