linux-kernel - Re: [PATCH] ASoC: meson: axg-fifo: fix irq scheduling issue with PREEMPT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240806123600.uI5LeCdp@linutronix.de>
Date: Tue, 6 Aug 2024 14:36:00 +0200
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Jerome Brunet <jbrunet@...libre.com>
Cc: Mark Brown <broonie@...nel.org>, Liam Girdwood <lgirdwood@...il.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	linux-amlogic@...ts.infradead.org, alsa-devel@...a-project.org,
	linux-kernel@...r.kernel.org,
	Arseniy Krasnov <avkrasnov@...utedevices.com>
Subject: Re: [PATCH] ASoC: meson: axg-fifo: fix irq scheduling issue with
 PREEMPT_RT

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@...utedevices.com>
> Closes: https://lore.kernel.org/linux-amlogic/20240729131652.3012327-1-avkrasnov@salutedevices.com
> Suggested-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> Fixes: b11d26660dff ("ASoC: meson: axg-fifo: use threaded irq to check periods")
> Signed-off-by: Jerome Brunet <jbrunet@...libre.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