1. Patchew
  2. linux
  3. [PATCH 0/7] media: renesas: rzv2h-ivc: Fix concurrent job scheduling
  4. View patch

[PATCH 7/7] media: rzv2h-ivc: Replace workqueue with direct function call

Jacopo Mondi posted 7 patches 3 weeks, 4 days ago
[PATCH 7/7] media: rzv2h-ivc: Replace workqueue with direct function call
Posted by Jacopo Mondi 3 weeks, 4 days ago 
From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>

Scheduling of work items with an async workqueue opens the door to
potential races between multiple instances of a work item.

While the frame transfer function is now protected agains races, using a
workqueue doesn't provide much benefit considering the limited cost of
creating a job transfer.

Replace usage of the work queue with direct function calls.

Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
 .../platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c      |  2 +-
 .../platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c    | 21 +++++++--------------
 .../media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h    |  3 +--
 3 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c
index e9857eb5b51a..355842abb24b 100644
--- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c
+++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c
@@ -119,7 +119,7 @@ static irqreturn_t rzv2h_ivc_isr(int irq, void *context)
 	 * The second interrupt indicates that the post-frame transfer VBLANK
 	 * has completed, we can now schedule a new frame transfer, if any.
 	 */
-	queue_work(ivc->buffers.async_wq, &ivc->buffers.work);
+	rzv2h_ivc_transfer_buffer(ivc);
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
index 3580a57738a6..b167f1bab7ef 100644
--- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
+++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
@@ -143,13 +143,11 @@ void rzv2h_ivc_buffer_done(struct rzv2h_ivc *ivc)
 	vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
 }
 
-static void rzv2h_ivc_transfer_buffer(struct work_struct *work)
+void rzv2h_ivc_transfer_buffer(struct rzv2h_ivc *ivc)
 {
-	struct rzv2h_ivc *ivc = container_of(work, struct rzv2h_ivc,
-					     buffers.work);
 	struct rzv2h_ivc_buf *buf;
 
-	guard(spinlock_irqsave)(&ivc->spinlock);
+	lockdep_assert_held(&ivc->spinlock);
 
 	if (ivc->vvalid_ifp)
 		return;
@@ -204,7 +202,7 @@ static void rzv2h_ivc_buf_queue(struct vb2_buffer *vb)
 
 	scoped_guard(spinlock_irq, &ivc->spinlock) {
 		if (vb2_is_streaming(vb->vb2_queue))
-			queue_work(ivc->buffers.async_wq, &ivc->buffers.work);
+			rzv2h_ivc_transfer_buffer(ivc);
 	}
 }
 
@@ -282,7 +280,9 @@ static int rzv2h_ivc_start_streaming(struct vb2_queue *q, unsigned int count)
 
 	rzv2h_ivc_format_configure(ivc);
 
-	queue_work(ivc->buffers.async_wq, &ivc->buffers.work);
+	scoped_guard(spinlock_irq, &ivc->spinlock) {
+		rzv2h_ivc_transfer_buffer(ivc);
+	}
 
 	return 0;
 
@@ -449,11 +449,6 @@ int rzv2h_ivc_init_vdev(struct rzv2h_ivc *ivc, struct v4l2_device *v4l2_dev)
 
 	spin_lock_init(&ivc->buffers.lock);
 	INIT_LIST_HEAD(&ivc->buffers.queue);
-	INIT_WORK(&ivc->buffers.work, rzv2h_ivc_transfer_buffer);
-
-	ivc->buffers.async_wq = alloc_workqueue("rzv2h-ivc", 0, 0);
-	if (!ivc->buffers.async_wq)
-		return -EINVAL;
 
 	/* Initialise vb2 queue */
 	vb2q = &ivc->vdev.vb2q;
@@ -471,7 +466,7 @@ int rzv2h_ivc_init_vdev(struct rzv2h_ivc *ivc, struct v4l2_device *v4l2_dev)
 	ret = vb2_queue_init(vb2q);
 	if (ret) {
 		dev_err(ivc->dev, "vb2 queue init failed\n");
-		goto err_destroy_workqueue;
+		return ret;
 	}
 
 	/* Initialise Video Device */
@@ -520,8 +515,6 @@ int rzv2h_ivc_init_vdev(struct rzv2h_ivc *ivc, struct v4l2_device *v4l2_dev)
 	media_entity_cleanup(&vdev->entity);
 err_release_vb2q:
 	vb2_queue_release(vb2q);
-err_destroy_workqueue:
-	destroy_workqueue(ivc->buffers.async_wq);
 
 	return ret;
 }
diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
index 049f223200e3..6f644ba796a9 100644
--- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
+++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
@@ -104,8 +104,6 @@ struct rzv2h_ivc {
 	struct {
 		/* Spinlock to guard buffer queue */
 		spinlock_t lock;
-		struct workqueue_struct *async_wq;
-		struct work_struct work;
 		struct list_head queue;
 		struct rzv2h_ivc_buf *curr;
 		unsigned int sequence;
@@ -130,3 +128,4 @@ void rzv2h_ivc_deinit_subdevice(struct rzv2h_ivc *ivc);
 void rzv2h_ivc_write(struct rzv2h_ivc *ivc, u32 addr, u32 val);
 void rzv2h_ivc_update_bits(struct rzv2h_ivc *ivc, unsigned int addr,
 			   u32 mask, u32 val);
+void rzv2h_ivc_transfer_buffer(struct rzv2h_ivc *ivc);

-- 
2.53.0
Re: [PATCH 7/7] media: rzv2h-ivc: Replace workqueue with direct function call
Posted by Dan Scally 3 weeks, 3 days ago 
Hi Jacopo

On 13/03/2026 11:14, Jacopo Mondi wrote:
> From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> 
> Scheduling of work items with an async workqueue opens the door to
> potential races between multiple instances of a work item.
> 
> While the frame transfer function is now protected agains races, using a
> workqueue doesn't provide much benefit considering the limited cost of
> creating a job transfer.
> 
> Replace usage of the work queue with direct function calls.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   .../platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c      |  2 +-
>   .../platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c    | 21 +++++++--------------
>   .../media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h    |  3 +--
>   3 files changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c
> index e9857eb5b51a..355842abb24b 100644
> --- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c
> +++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-dev.c
> @@ -119,7 +119,7 @@ static irqreturn_t rzv2h_ivc_isr(int irq, void *context)
>   	 * The second interrupt indicates that the post-frame transfer VBLANK
>   	 * has completed, we can now schedule a new frame transfer, if any.
>   	 */
> -	queue_work(ivc->buffers.async_wq, &ivc->buffers.work);
> +	rzv2h_ivc_transfer_buffer(ivc);
>   
>   	return IRQ_HANDLED;
>   }
> diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> index 3580a57738a6..b167f1bab7ef 100644
> --- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> +++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> @@ -143,13 +143,11 @@ void rzv2h_ivc_buffer_done(struct rzv2h_ivc *ivc)
>   	vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
>   }
>   
> -static void rzv2h_ivc_transfer_buffer(struct work_struct *work)
> +void rzv2h_ivc_transfer_buffer(struct rzv2h_ivc *ivc)
>   {
> -	struct rzv2h_ivc *ivc = container_of(work, struct rzv2h_ivc,
> -					     buffers.work);
>   	struct rzv2h_ivc_buf *buf;
>   
> -	guard(spinlock_irqsave)(&ivc->spinlock);
> +	lockdep_assert_held(&ivc->spinlock);
>   
>   	if (ivc->vvalid_ifp)
>   		return;
> @@ -204,7 +202,7 @@ static void rzv2h_ivc_buf_queue(struct vb2_buffer *vb)
>   
>   	scoped_guard(spinlock_irq, &ivc->spinlock) {
>   		if (vb2_is_streaming(vb->vb2_queue))
> -			queue_work(ivc->buffers.async_wq, &ivc->buffers.work);
> +			rzv2h_ivc_transfer_buffer(ivc);
>   	}
>   }
>   
> @@ -282,7 +280,9 @@ static int rzv2h_ivc_start_streaming(struct vb2_queue *q, unsigned int count)
>   
>   	rzv2h_ivc_format_configure(ivc);
>   
> -	queue_work(ivc->buffers.async_wq, &ivc->buffers.work);
> +	scoped_guard(spinlock_irq, &ivc->spinlock) {
> +		rzv2h_ivc_transfer_buffer(ivc);
> +	}
>   
>   	return 0;
>   
> @@ -449,11 +449,6 @@ int rzv2h_ivc_init_vdev(struct rzv2h_ivc *ivc, struct v4l2_device *v4l2_dev)
>   
>   	spin_lock_init(&ivc->buffers.lock);
>   	INIT_LIST_HEAD(&ivc->buffers.queue);
> -	INIT_WORK(&ivc->buffers.work, rzv2h_ivc_transfer_buffer);
> -
> -	ivc->buffers.async_wq = alloc_workqueue("rzv2h-ivc", 0, 0);
> -	if (!ivc->buffers.async_wq)
> -		return -EINVAL;
>   
>   	/* Initialise vb2 queue */
>   	vb2q = &ivc->vdev.vb2q;
> @@ -471,7 +466,7 @@ int rzv2h_ivc_init_vdev(struct rzv2h_ivc *ivc, struct v4l2_device *v4l2_dev)
>   	ret = vb2_queue_init(vb2q);
>   	if (ret) {
>   		dev_err(ivc->dev, "vb2 queue init failed\n");
> -		goto err_destroy_workqueue;
> +		return ret;
>   	}
>   
>   	/* Initialise Video Device */
> @@ -520,8 +515,6 @@ int rzv2h_ivc_init_vdev(struct rzv2h_ivc *ivc, struct v4l2_device *v4l2_dev)
>   	media_entity_cleanup(&vdev->entity);
>   err_release_vb2q:
>   	vb2_queue_release(vb2q);
> -err_destroy_workqueue:
> -	destroy_workqueue(ivc->buffers.async_wq);
>   
>   	return ret;
>   }
> diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> index 049f223200e3..6f644ba796a9 100644
> --- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> +++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> @@ -104,8 +104,6 @@ struct rzv2h_ivc {
>   	struct {
>   		/* Spinlock to guard buffer queue */
>   		spinlock_t lock;
> -		struct workqueue_struct *async_wq;
> -		struct work_struct work;
>   		struct list_head queue;
>   		struct rzv2h_ivc_buf *curr;
>   		unsigned int sequence;
> @@ -130,3 +128,4 @@ void rzv2h_ivc_deinit_subdevice(struct rzv2h_ivc *ivc);
>   void rzv2h_ivc_write(struct rzv2h_ivc *ivc, u32 addr, u32 val);
>   void rzv2h_ivc_update_bits(struct rzv2h_ivc *ivc, unsigned int addr,
>   			   u32 mask, u32 val);
> +void rzv2h_ivc_transfer_buffer(struct rzv2h_ivc *ivc);
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.