| 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
| ||
|
Message-ID: <CAAEAJfALjVhrLcVbS0vyXXPJUy1tmv4PcMbWxte0u9htn+myjw@mail.gmail.com>
Date: Sun, 6 Feb 2022 10:09:11 -0300
From: Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
To: Jernej Skrabec <jernej.skrabec@...il.com>
Cc: Maxime Ripard <mripard@...nel.org>,
Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Chen-Yu Tsai <wens@...e.org>,
linux-media <linux-media@...r.kernel.org>,
"open list:STAGING SUBSYSTEM" <linux-staging@...ts.linux.dev>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
linux-sunxi@...ts.linux.dev,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] media: cedrus: Add watchdog for job completion
Hello,
On Tue, 1 Feb 2022 at 15:34, Jernej Skrabec <jernej.skrabec@...il.com> wrote:
>
> Currently, if job is not completed for whatever reason, userspace
> application can hang on ioctl and thus become unkillable.
>
> In order to prevent that, implement watchdog, which will complete job
> after 2 seconds with error state.
>
> Concept is borrowed from hantro driver.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@...il.com>
This has been very useful developing Hantro,
to get some indication of hardware being programmed wrongly:
Patch looks correct.
Reviewed-by: Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.c | 2 ++
> drivers/staging/media/sunxi/cedrus/cedrus.h | 3 +++
> .../staging/media/sunxi/cedrus/cedrus_dec.c | 4 +++
> .../staging/media/sunxi/cedrus/cedrus_hw.c | 25 +++++++++++++++++++
> .../staging/media/sunxi/cedrus/cedrus_hw.h | 2 ++
> 5 files changed, 36 insertions(+)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 4a4b714b0f26..68b3dcdb5df3 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -439,6 +439,8 @@ static int cedrus_probe(struct platform_device *pdev)
>
> mutex_init(&dev->dev_mutex);
>
> + INIT_DELAYED_WORK(&dev->watchdog_work, cedrus_watchdog);
> +
> ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> if (ret) {
> dev_err(&pdev->dev, "Failed to register V4L2 device\n");
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index c345f2984041..3bc094eb497f 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -24,6 +24,7 @@
>
> #include <linux/iopoll.h>
> #include <linux/platform_device.h>
> +#include <linux/workqueue.h>
>
> #define CEDRUS_NAME "cedrus"
>
> @@ -194,6 +195,8 @@ struct cedrus_dev {
> struct reset_control *rstc;
>
> unsigned int capabilities;
> +
> + struct delayed_work watchdog_work;
> };
>
> extern struct cedrus_dec_ops cedrus_dec_ops_mpeg2;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index a16c1422558f..9c7200299465 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -97,4 +97,8 @@ void cedrus_device_run(void *priv)
> v4l2_ctrl_request_complete(src_req, &ctx->hdl);
>
> dev->dec_ops[ctx->current_codec]->trigger(ctx);
> +
> + /* Start the watchdog timer. */
> + schedule_delayed_work(&dev->watchdog_work,
> + msecs_to_jiffies(2000));
> }
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> index 2d7663726467..a6470a89851e 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> @@ -118,6 +118,13 @@ static irqreturn_t cedrus_irq(int irq, void *data)
> enum vb2_buffer_state state;
> enum cedrus_irq_status status;
>
> + /*
> + * If cancel_delayed_work returns false it means watchdog already
> + * executed and finished the job.
> + */
> + if (!cancel_delayed_work(&dev->watchdog_work))
> + return IRQ_HANDLED;
> +
> ctx = v4l2_m2m_get_curr_priv(dev->m2m_dev);
> if (!ctx) {
> v4l2_err(&dev->v4l2_dev,
> @@ -143,6 +150,24 @@ static irqreturn_t cedrus_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +void cedrus_watchdog(struct work_struct *work)
> +{
> + struct cedrus_dev *dev;
> + struct cedrus_ctx *ctx;
> +
> + dev = container_of(to_delayed_work(work),
> + struct cedrus_dev, watchdog_work);
> +
> + ctx = v4l2_m2m_get_curr_priv(dev->m2m_dev);
> + if (!ctx)
> + return;
> +
> + v4l2_err(&dev->v4l2_dev, "frame processing timed out!\n");
> + reset_control_reset(dev->rstc);
> + v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,
> + VB2_BUF_STATE_ERROR);
> +}
> +
> int cedrus_hw_suspend(struct device *device)
> {
> struct cedrus_dev *dev = dev_get_drvdata(device);
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> index 45f641f0bfa2..7c92f00e36da 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
> @@ -28,4 +28,6 @@ int cedrus_hw_resume(struct device *device);
> int cedrus_hw_probe(struct cedrus_dev *dev);
> void cedrus_hw_remove(struct cedrus_dev *dev);
>
> +void cedrus_watchdog(struct work_struct *work);
> +
> #endif
> --
> 2.35.1
>
Powered by blists - more mailing lists