Re: [PATCH] media: msi2500: return buffers as QUEUED in buf_queue() before start_streaming()

[zhangdaolong]

On 11/11/2026 20:29, Hans Verkuil wrote:
> On 06/11/2026 09:41, zhangdaolong wrote:
>> diff --git a/drivers/media/usb/msi2500/msi2500.c b/drivers/media/usb/msi2500/msi2500.c
>> index 282256ab812a..d89db57f924f 100644
>> --- a/drivers/media/usb/msi2500/msi2500.c
>> +++ b/drivers/media/usb/msi2500/msi2500.c
>> @@ -625,7 +625,9 @@ static void msi2500_buf_queue(struct vb2_buffer *vb)
>>  
>>  	/* Check the device has not disconnected between prep and queuing */
>>  	if (unlikely(!dev->udev)) {
>> -		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
>> +		vb2_buffer_done(&buf->vb.vb2_buf,
>> +				vb2_start_streaming_called(vb->vb2_queue) ?
>> +				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_QUEUED);
>>  		return;
>>  	}
> 
> I think the real problem is that msi2500_buf_queue is calling vb2_buffer_done
> at all.
> 
> What happens if you just remove this 'if' completely?
> 
> I don't think I have seen vb2_buffer_done being called from buf_queue before.
> It's very dubious code.

Hi Hans,

You are right. Removing this 'if' from msi2500_buf_queue() completely works
cleanly and is a much better fix.

This check was originally copied from the pwc driver (commit c20d78cde370
("[media] pwc: Rework locking")), where vb2 operations were briefly called
without holding a queue lock. Once vb_queue_lock was added, the check became
obsolete, and msi2500 doesn't even implement buf_prepare().

With this 'if' removed:
- Normal ioctls (e.g. VIDIOC_QBUF, VIDIOC_STREAMON) check
  video_is_registered() under vb_queue_lock in __video_do_ioctl(), while
  msi2500_disconnect() sets dev->udev = NULL and unregisters the video
  device under the same vb_queue_lock, so they cannot reach buf_queue()
  after disconnect.
- When v4l2_read() races with msi2500_disconnect() before vb2_fop_read()
  acquires vb_queue_lock, msi2500_buf_queue() simply adds the buffers to
  dev->queued_bufs, and msi2500_start_streaming() immediately catches
  !dev->udev, returns all buffers on dev->queued_bufs with
  VB2_BUF_STATE_QUEUED via msi2500_cleanup_queued_bufs(), and fails with
  -ENODEV.

I will send a v2 patch that removes this check from msi2500_buf_queue().

Thanks,
Daolong