linux-kernel - Re: [PATCH v1] ASoC: meson: axg-fifo: set option to use raw spinlock

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: <20240806065021.PINvpze_@linutronix.de>
Date: Tue, 6 Aug 2024 08:50:21 +0200
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Jerome Brunet <jbrunet@...libre.com>
Cc: Mark Brown <broonie@...nel.org>, linux-rt-users@...r.kernel.org,
	Arseniy Krasnov <avkrasnov@...utedevices.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Kevin Hilman <khilman@...libre.com>,
	Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
	alsa-devel@...a-project.org, linux-sound@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-amlogic@...ts.infradead.org, linux-kernel@...r.kernel.org,
	kernel@...rdevices.ru, oxffffaa@...il.com,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v1] ASoC: meson: axg-fifo: set option to use raw spinlock

On 2024-08-05 18:07:28 [+0200], Jerome Brunet wrote:
> Hi Sebastian,
Hi Jerome,

> Thanks a lot for taking the time to dig into the driver specifics.
> The IRQ handler is actually not awfully critical in this case. The HW
> will continue to run regardless of the IRQ being acked or not
> 
> The purpose of the handler is mainly to let Alsa know that 1 (or more)
> period is ready. If alsa is too slow to react, and the buffer allows
> just a few periods, the HW may under/overflow the buffer.
> 
> IRQF_ONESHOT is fine because snd_pcm_period_elapsed() 'notifies'
> all past periods, not just one. The irq handler does not need to
> run again until this function has been called. It also helps if the
> period is ridiculously small, to try to reduce the number of IRQs.

IRQF_ONESHOT is used to disable to keep the IRQ line disabled (after the
primary handler) while the threaded handler is running. This implies
that the primary handler must not be threaded under PREEMPT_RT.
…
> I'd prefer #1 too. IRQ is not shared, so the ownership should be fine.
> 
> However I still don't fully understand what we are fixing here TBH. It
> seems it could apply to other parts of the kernel so I'd like to
> understand what is wrong (and avoid repeating the same mistake)
> 
> * With PREEMPT_RT, both handler will threaded, so they should be able to
>   call preemptible function, right ?

Correct. But flags like IRQF_ONESHOT won't thread the primary handler.

> * If so, and spinlock_lock() actually becomes preemptible with
>   PREEMPT_RT as stated in this change description, why is it problem here ?

Because in this case the primary handler is not threaded and runs in
not preemptible hard-IRQ context.

> Do you have an idea about what is going on ?

Yes.

Sebastian