linux-kernel - Re: [PATCH v2 3/3] media: uvcvideo: Remove stream->is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6851f402-d987-4cfb-a18e-193d3a6fe3cb@jjverkuil.nl>
Date: Mon, 2 Jun 2025 18:07:39 +0200
From: Hans Verkuil <hans@...erkuil.nl>
To: Ricardo Ribalda <ribalda@...omium.org>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Hans de Goede <hdegoede@...hat.com>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] media: uvcvideo: Remove stream->is_streaming field

On 02/06/2025 16:38, Ricardo Ribalda wrote:
> Hi Laurent, Hi Hans
>>> If this isn't fixed, then at least add a comment explaining why you test for
>>> != V4L2_BUF_TYPE_META_CAPTURE before calling uvc_pm_put. It's not obvious.
>>
>> Agreed.
> 
> Maybe this is better than a comment?

Yes, that's better.

Regards,

	Hans

> 
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c
> index 72c5494dee9f..7f9d731df32c 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -39,8 +39,6 @@ static inline struct uvc_buffer
> *uvc_vbuf_to_buffer(struct vb2_v4l2_buffer *buf)
> 
>  /*
>   * Return all queued buffers to videobuf2 in the requested state.
> - *
> - * This function must be called with the queue spinlock held.
>   */
>  static void uvc_queue_return_buffers(struct uvc_video_queue *queue,
>                                enum uvc_buffer_state state)
> @@ -49,6 +47,8 @@ static void uvc_queue_return_buffers(struct
> uvc_video_queue *queue,
>                                         ? VB2_BUF_STATE_ERROR
>                                         : VB2_BUF_STATE_QUEUED;
> 
> +       spin_lock_irq(&queue->irqlock);
> +
>         while (!list_empty(&queue->irqqueue)) {
>                 struct uvc_buffer *buf = list_first_entry(&queue->irqqueue,
>                                                           struct uvc_buffer,
> @@ -57,6 +57,8 @@ static void uvc_queue_return_buffers(struct
> uvc_video_queue *queue,
>                 buf->state = state;
>                 vb2_buffer_done(&buf->buf.vb2_buf, vb2_state);
>         }
> +
> +       spin_unlock_irq(&queue->irqlock);
>  }
> 
>  /* -----------------------------------------------------------------------------
> @@ -157,7 +159,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
>                 uvc_video_clock_update(stream, vbuf, buf);
>  }
> 
> -static int uvc_start_streaming(struct vb2_queue *vq, unsigned int count)
> +static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
>  {
>         struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>         struct uvc_streaming *stream = uvc_queue_to_stream(queue);
> @@ -171,25 +173,29 @@ static int uvc_start_streaming(struct vb2_queue
> *vq, unsigned int count)
>         if (ret == 0)
>                 return 0;
> 
> -       spin_lock_irq(&queue->irqlock);
>         uvc_queue_return_buffers(queue, UVC_BUF_STATE_QUEUED);
> -       spin_unlock_irq(&queue->irqlock);
> 
>         return ret;
>  }
> 
> -static void uvc_stop_streaming(struct vb2_queue *vq)
> +static void uvc_stop_streaming_meta(struct vb2_queue *vq)
>  {
>         struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> 
>         lockdep_assert_irqs_enabled();
> 
> -       if (vq->type != V4L2_BUF_TYPE_META_CAPTURE)
> -               uvc_video_stop_streaming(uvc_queue_to_stream(queue));
> +       uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> +}
> +
> +static void uvc_stop_streaming_video(struct vb2_queue *vq)
> +{
> +       struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
> +
> +       lockdep_assert_irqs_enabled();
> +
> +       uvc_video_stop_streaming(uvc_queue_to_stream(queue));
> 
> -       spin_lock_irq(&queue->irqlock);
>         uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> -       spin_unlock_irq(&queue->irqlock);
>  }
> 
>  static const struct vb2_ops uvc_queue_qops = {
> @@ -197,15 +203,15 @@ static const struct vb2_ops uvc_queue_qops = {
>         .buf_prepare = uvc_buffer_prepare,
>         .buf_queue = uvc_buffer_queue,
>         .buf_finish = uvc_buffer_finish,
> -       .start_streaming = uvc_start_streaming,
> -       .stop_streaming = uvc_stop_streaming,
> +       .start_streaming = uvc_start_streaming_video,
> +       .stop_streaming = uvc_stop_streaming_video,
>  };
> 
>  static const struct vb2_ops uvc_meta_queue_qops = {
>         .queue_setup = uvc_queue_setup,
>         .buf_prepare = uvc_buffer_prepare,
>         .buf_queue = uvc_buffer_queue,
> -       .stop_streaming = uvc_stop_streaming,
> +       .stop_streaming = uvc_stop_streaming_meta,
>  };
> 
>  int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type)
> 
> 
>