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

[Dan Scally <dan.scally@ideasonboard.com>]

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