[PATCH] media: hws: Wait for IRQ handler before returning buffers

[PATCH] media: hws: Wait for IRQ handler before returning buffers

[Hao-Qun Huang]

hws_stop_streaming() disables capture and then collects the active and
queued buffers straight away. Clearing cap_active and setting
stop_requested only stops a VDONE handler that has not checked them
yet; one already running on another CPU has passed those checks and
cannot be recalled.

That handler snapshots v->active into a local pointer and drops
irq_lock before it touches the buffer, so stop_streaming can run in
between. Without a next_prepared buffer both paths complete the same
buffer, and the second vb2_buffer_done() hits the WARN_ON for a buffer
that is no longer active. With a next_prepared buffer the snapshot is
the only remaining reference to the old active buffer, so
stop_streaming returns without it and vb2 reports "stop_streaming
operation is leaving buffer %u in active state" before completing it
with an error.

Either way the driver breaks the vb2 rule that stop_streaming has to
give back every buffer it owns before it returns.

Wait for the handler once the hardware is disabled and before the
buffers are collected. The live mode change and the channel cleanup
paths already do this around the same collect helper.

Fixes: ba07fd2f5742 ("media: pci: add AVMatrix HWS capture driver")
Assisted-by: Claude:claude-opus-5
Signed-off-by: Hao-Qun Huang <alvinhuang0603@gmail.com>
---
Found by code inspection; I do not have an HWS card, so this is not
reproduced on hardware. What convinced me is the asymmetry inside the
driver itself: the live mode change path calls synchronize_irq() before
the same hws_video_collect_done_locked() helper, and hws_stop_streaming()
does not.

 drivers/media/pci/hws/hws_video.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/pci/hws/hws_video.c b/drivers/media/pci/hws/hws_video.c
index 18e4bc6901d3..7f7e51040926 100644
--- a/drivers/media/pci/hws/hws_video.c
+++ b/drivers/media/pci/hws/hws_video.c
@@ -1292,6 +1292,8 @@ static void hws_stop_streaming(struct vb2_queue *q)
 	WRITE_ONCE(v->stop_requested, true);
 
 	hws_enable_video_capture(v->parent, v->channel_index, false);
+	if (hws->irq >= 0)
+		synchronize_irq(hws->irq);
 
 	/* 2) Collect in-flight + queued under the IRQ lock */
 	spin_lock_irqsave(&v->irq_lock, flags);
-- 
2.43.0

Re: [PATCH] media: hws: Wait for IRQ handler before returning buffers

[Ben Hoff]

I reproduced this on HWS hardware.

Without the patch, the synchronized STREAMOFF/VDONE test produced vb2
ownership warnings. With the patch applied, 500 attempts entered the
instrumented 250 ms post-snapshot VDONE window without an ownership
warning or capture failure.

Reviewed-by: Ben Hoff <hoff.benjamin.k@gmail.com>
Tested-by: Ben Hoff <hoff.benjamin.k@gmail.com>

On Sat, Aug 8, 2026 at 11:08 AM Hao-Qun Huang <alvinhuang0603@gmail.com> wrote:
>
> hws_stop_streaming() disables capture and then collects the active and
> queued buffers straight away. Clearing cap_active and setting
> stop_requested only stops a VDONE handler that has not checked them
> yet; one already running on another CPU has passed those checks and
> cannot be recalled.
>
> That handler snapshots v->active into a local pointer and drops
> irq_lock before it touches the buffer, so stop_streaming can run in
> between. Without a next_prepared buffer both paths complete the same
> buffer, and the second vb2_buffer_done() hits the WARN_ON for a buffer
> that is no longer active. With a next_prepared buffer the snapshot is
> the only remaining reference to the old active buffer, so
> stop_streaming returns without it and vb2 reports "stop_streaming
> operation is leaving buffer %u in active state" before completing it
> with an error.
>
> Either way the driver breaks the vb2 rule that stop_streaming has to
> give back every buffer it owns before it returns.
>
> Wait for the handler once the hardware is disabled and before the
> buffers are collected. The live mode change and the channel cleanup
> paths already do this around the same collect helper.
>
> Fixes: ba07fd2f5742 ("media: pci: add AVMatrix HWS capture driver")
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Hao-Qun Huang <alvinhuang0603@gmail.com>
> ---
> Found by code inspection; I do not have an HWS card, so this is not
> reproduced on hardware. What convinced me is the asymmetry inside the
> driver itself: the live mode change path calls synchronize_irq() before
> the same hws_video_collect_done_locked() helper, and hws_stop_streaming()
> does not.
>
>  drivers/media/pci/hws/hws_video.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/pci/hws/hws_video.c b/drivers/media/pci/hws/hws_video.c
> index 18e4bc6901d3..7f7e51040926 100644
> --- a/drivers/media/pci/hws/hws_video.c
> +++ b/drivers/media/pci/hws/hws_video.c
> @@ -1292,6 +1292,8 @@ static void hws_stop_streaming(struct vb2_queue *q)
>         WRITE_ONCE(v->stop_requested, true);
>
>         hws_enable_video_capture(v->parent, v->channel_index, false);
> +       if (hws->irq >= 0)
> +               synchronize_irq(hws->irq);
>
>         /* 2) Collect in-flight + queued under the IRQ lock */
>         spin_lock_irqsave(&v->irq_lock, flags);
> --
> 2.43.0
>