Re: [PATCH] ASoC: meson: axg-fifo: fix irq scheduling issue with PREEMPT_RT

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2024-08-06 12:27:03 [+0200], Jerome Brunet wrote:
> With PREEMPT_RT enabled, spinlocks become preemptible.
With PREEMPT_RT enabled a spinlock_t becomes a sleeping lock.

> This is usually not a problem with spinlocks used in IRQ context since
> IRQ handlers get threaded. However, if IRQF_ONESHOT is set, the upper half
> of a threaded irq handler won't be threaded and this causes scheduling
> problems if spinlocks are used in the upper half, like with regmap when
> '.fast_io' is set.

… However, if IRQF_ONESHOT is set, the primary handler won't be
force-threaded and runs always in hardirq context. This is a problem
because spinlock_t requires a preemptible context on PREEMPT_RT. 

> In this particular instance, it is actually better to do everything in
> the bottom half and it solves the problem with PREEMPT_RT.

      threaded handler

The term "bottom half" is usually used with softirq. The IRQ part has a
primary handler and a threaded handler.

> Reported-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
> Closes: https://lore.kernel.org/linux-amlogic/20240729131652.3012327-1-avkrasnov@salutedevices.com
> Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Fixes: b11d26660dff ("ASoC: meson: axg-fifo: use threaded irq to check periods")
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  sound/soc/meson/axg-fifo.c | 26 ++++++++++----------------
>  1 file changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/sound/soc/meson/axg-fifo.c b/sound/soc/meson/axg-fifo.c
> index 7e6090af720b..d0d05fa44d66 100644
> --- a/sound/soc/meson/axg-fifo.c
> +++ b/sound/soc/meson/axg-fifo.c
> @@ -251,8 +244,9 @@ int axg_fifo_pcm_open(struct snd_soc_component *component,
>  	if (ret)
>  		return ret;
>  
> -	ret = request_threaded_irq(fifo->irq, axg_fifo_pcm_irq_block,
> -				   axg_fifo_pcm_irq_block_thread,
> +	/* Use the bottom half of a threaded irq with non-atomic links */

This requests only a threaded handler. There is no "bottom half of a
threaded IRQ".

> +	ret = request_threaded_irq(fifo->irq, NULL,
> +				   axg_fifo_pcm_irq_block,
>  				   IRQF_ONESHOT, dev_name(dev), ss);
>  	if (ret)
>  		return ret;

Sebastian

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic