On Thu, 17 Sep 2020 14:03:33 +0200
Christian Eggers <cegg...@arri.de> wrote:
> Triggers may raise transactions on slow busses like I2C. Using the
> original RT priority of a threaded IRQ may prevent other important IRQ
> handlers from being run.
>
> Signed-off-by: Christian Eggers <cegg...@arri.de>
> Cc: sta...@vger.kernel.org
> ---
> In my particular case (on a RT kernel), the RT priority of the sysfstrig
> threaded IRQ handler caused (temporarily) raising the prio of a user
> space process which was holding the I2C bus mutex.
>
> Due to a bug in the i2c-imx driver, this process spent 500 ms in a busy-wait
> loop and prevented all threaded IRQ handlers from being run during this
> time.
I'm not sure I fully understand the impacts of this yet.
What is the impact on cases where we don't have any nasty side affects
due to users of the trigger?
I presume reducing the priority will cause some reduction in
performance? If so is there any chance that would count as a regression?
Jonathan
>
> v2:
> - Use sched_set_normal() instead of sched_setscheduler_nocheck()
>
> drivers/iio/industrialio-trigger.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/iio/industrialio-trigger.c
> b/drivers/iio/industrialio-trigger.c
> index 6f16357fd732..7ed00ad695c7 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -9,7 +9,10 @@
> #include <linux/err.h>
> #include <linux/device.h>
> #include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqdesc.h>
> #include <linux/list.h>
> +#include <linux/sched.h>
> #include <linux/slab.h>
>
> #include <linux/iio/iio.h>
> @@ -245,6 +248,7 @@ int iio_trigger_attach_poll_func(struct iio_trigger *trig,
> int ret = 0;
> bool notinuse
> = bitmap_empty(trig->pool, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
> + struct irq_desc *irq_desc;
>
> /* Prevent the module from being removed whilst attached to a trigger */
> __module_get(pf->indio_dev->driver_module);
> @@ -264,6 +268,12 @@ int iio_trigger_attach_poll_func(struct iio_trigger
> *trig,
> if (ret < 0)
> goto out_put_irq;
>
> + /* Triggers may raise transactions on slow busses like I2C. Using the
> original RT priority
> + * of a threaded IRQ may prevent other threaded IRQ handlers from being
> run.
> + */
> + irq_desc = irq_to_desc(pf->irq);
> + sched_set_normal(irq_desc->action->thread, 0);
> +
> /* Enable trigger in driver */
> if (trig->ops && trig->ops->set_trigger_state && notinuse) {
> ret = trig->ops->set_trigger_state(trig, true);
