The watchdog needs to be schedule before we trigger the decode operation, otherwise there is a risk that the decoder IRQ will be called before we have schedule the watchdog. As a side effect, the watchdog would never be cancelled and its function would be called at an inappropriate time.
This was observed while running Fluster with GStreamer as a backend. Some programming error would cause the decoder IRQ to be call very quickly after the trigger. Later calls into the driver would deadlock due to the unbalanced state.
Cc: stable@vger.kernel.org Fixes: 7c38a551bda1 ("media: cedrus: Add watchdog for job completion") Signed-off-by: Nicolas Dufresne nicolas.dufresne@collabora.com --- drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index 3b6aa78a2985f..e7f7602a5ab40 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c @@ -106,11 +106,11 @@ void cedrus_device_run(void *priv)
/* Trigger decoding if setup went well, bail out otherwise. */ if (!error) { - dev->dec_ops[ctx->current_codec]->trigger(ctx); - /* Start the watchdog timer. */ schedule_delayed_work(&dev->watchdog_work, msecs_to_jiffies(2000)); + + dev->dec_ops[ctx->current_codec]->trigger(ctx); } else { v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,
Hi Nicolas,
On Thu 18 Aug 22, 16:33, Nicolas Dufresne wrote:
The watchdog needs to be schedule before we trigger the decode operation, otherwise there is a risk that the decoder IRQ will be called before we have schedule the watchdog. As a side effect, the watchdog would never be cancelled and its function would be called at an inappropriate time.
This was observed while running Fluster with GStreamer as a backend. Some programming error would cause the decoder IRQ to be call very quickly after the trigger. Later calls into the driver would deadlock due to the unbalanced state.
Good catch, thanks!
Reviewed-by: Paul Kocialkowski paul.kocialkowski@bootlin.com
Cheers,
Paul
Cc: stable@vger.kernel.org Fixes: 7c38a551bda1 ("media: cedrus: Add watchdog for job completion") Signed-off-by: Nicolas Dufresne nicolas.dufresne@collabora.com
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index 3b6aa78a2985f..e7f7602a5ab40 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c @@ -106,11 +106,11 @@ void cedrus_device_run(void *priv) /* Trigger decoding if setup went well, bail out otherwise. */ if (!error) {
dev->dec_ops[ctx->current_codec]->trigger(ctx);
- /* Start the watchdog timer. */ schedule_delayed_work(&dev->watchdog_work, msecs_to_jiffies(2000));
} else { v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,dev->dec_ops[ctx->current_codec]->trigger(ctx);
-- 2.37.2
Dne četrtek, 18. avgust 2022 ob 22:33:06 CEST je Nicolas Dufresne napisal(a):
The watchdog needs to be schedule before we trigger the decode operation, otherwise there is a risk that the decoder IRQ will be called before we have schedule the watchdog. As a side effect, the watchdog would never be cancelled and its function would be called at an inappropriate time.
This was observed while running Fluster with GStreamer as a backend. Some programming error would cause the decoder IRQ to be call very quickly after the trigger. Later calls into the driver would deadlock due to the unbalanced state.
Cc: stable@vger.kernel.org Fixes: 7c38a551bda1 ("media: cedrus: Add watchdog for job completion") Signed-off-by: Nicolas Dufresne nicolas.dufresne@collabora.com
Reviewed-by: Jernej Skrabec jernej.skrabec@gmail.com
Best regards, Jernej
drivers/staging/media/sunxi/cedrus/cedrus_dec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index 3b6aa78a2985f..e7f7602a5ab40 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c @@ -106,11 +106,11 @@ void cedrus_device_run(void *priv)
/* Trigger decoding if setup went well, bail out otherwise. */ if (!error) {
dev->dec_ops[ctx->current_codec]->trigger(ctx);
- /* Start the watchdog timer. */ schedule_delayed_work(&dev->watchdog_work, msecs_to_jiffies(2000));
} else { v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx-dev->dec_ops[ctx->current_codec]->trigger(ctx);
fh.m2m_ctx,
2.37.2
linux-stable-mirror@lists.linaro.org