[PATCH] serial: stm32: Move hard IRQ handling to threaded interrupt context

[Valentin CARON]

Hi Marek,

I've got a patch in the same spirit in downstream.
The test campaign reveals performance issues with this patch.

In fact, hard IRQ have been introduced in stm32-usart driver to solve 
performance issues due to short FIFO size (16 bytes).

We are currently testing another patch, similar as your RFC proposition, 
for RT context.
But results are not ready yet. We can wait them before merging this big 
change into driver ?

Thanks,
Valentin

On 12/7/22 20:59, Marek Vasut wrote:
> Avoid locking in hard interrupt context, move the entirety of hard IRQ
> context code into the threaded IRQ handler. This fixes the following
> splat with preempt-rt enabled:
>
>   BUG: scheduling while atomic: (mount)/1289/0x00010001
>   Modules linked in:
>   Preemption disabled at:
>   [<c0119127>] irq_enter_rcu+0xb/0x42
>   CPU: 0 PID: 1289 Comm: (mount) Not tainted 6.1.0-rc7-rt5-stable-standard-00006-gd70aeccb9f0f #17
>   Hardware name: STM32 (Device Tree Support)
>    unwind_backtrace from show_stack+0xb/0xc
>    show_stack from dump_stack_lvl+0x2b/0x34
>    dump_stack_lvl from __schedule_bug+0x53/0x80
>    __schedule_bug from __schedule+0x47/0x404
>    __schedule from schedule_rtlock+0x15/0x34
>    schedule_rtlock from rtlock_slowlock_locked+0x1d7/0x57e
>    rtlock_slowlock_locked from rt_spin_lock+0x29/0x3c
>    rt_spin_lock from stm32_usart_interrupt+0xa9/0x110
>    stm32_usart_interrupt from __handle_irq_event_percpu+0x73/0x14e
>    __handle_irq_event_percpu from handle_irq_event_percpu+0x9/0x22
>    handle_irq_event_percpu from handle_irq_event+0x53/0x76
>    handle_irq_event from handle_fasteoi_irq+0x65/0xa8
>    handle_fasteoi_irq from handle_irq_desc+0xf/0x18
>    handle_irq_desc from gic_handle_irq+0x45/0x54
>    gic_handle_irq from generic_handle_arch_irq+0x19/0x2c
>    generic_handle_arch_irq from call_with_stack+0xd/0x10
>
> Signed-off-by: Marek Vasut <marex at denx.de>
> ---
> Cc: Alexandre Torgue <alexandre.torgue at foss.st.com>
> Cc: Erwan Le Ray <erwan.leray at foss.st.com>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> Cc: Jean Philippe Romain <jean-philippe.romain at foss.st.com>
> Cc: Jiri Slaby <jirislaby at kernel.org>
> Cc: Maxime Coquelin <mcoquelin.stm32 at gmail.com>
> Cc: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
> Cc: Thomas Gleixner <tglx at linutronix.de>
> Cc: Valentin Caron <valentin.caron at foss.st.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-stm32 at st-md-mailman.stormreply.com
> To: linux-serial at vger.kernel.org
> ---
>   drivers/tty/serial/stm32-usart.c | 27 +++++++--------------------
>   1 file changed, 7 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> index a1490033aa164..f5bce0be02676 100644
> --- a/drivers/tty/serial/stm32-usart.c
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -745,14 +745,15 @@ static void stm32_usart_transmit_chars(struct uart_port *port)
>   	}
>   }
>   
> -static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
> +static irqreturn_t stm32_usart_threaded_interrupt(int irq, void *ptr)
>   {
>   	struct uart_port *port = ptr;
>   	struct tty_port *tport = &port->state->port;
>   	struct stm32_port *stm32_port = to_stm32_port(port);
>   	const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> -	u32 sr;
> +	unsigned long flags;
>   	unsigned int size;
> +	u32 sr;
>   
>   	sr = readl_relaxed(port->membase + ofs->isr);
>   
> @@ -792,27 +793,13 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
>   	}
>   
>   	if ((sr & USART_SR_TXE) && !(stm32_port->tx_ch)) {
> -		spin_lock(&port->lock);
> +		spin_lock_irqsave(&port->lock, flags);
>   		stm32_usart_transmit_chars(port);
> -		spin_unlock(&port->lock);
> +		spin_unlock_irqrestore(&port->lock, flags);
>   	}
>   
> -	if (stm32_usart_rx_dma_enabled(port))
> -		return IRQ_WAKE_THREAD;
> -	else
> -		return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t stm32_usart_threaded_interrupt(int irq, void *ptr)
> -{
> -	struct uart_port *port = ptr;
> -	struct tty_port *tport = &port->state->port;
> -	struct stm32_port *stm32_port = to_stm32_port(port);
> -	unsigned int size;
> -	unsigned long flags;
> -
>   	/* Receiver timeout irq for DMA RX */
> -	if (!stm32_port->throttled) {
> +	if (stm32_usart_rx_dma_enabled(port) && !stm32_port->throttled) {
>   		spin_lock_irqsave(&port->lock, flags);
>   		size = stm32_usart_receive_chars(port, false);
>   		uart_unlock_and_check_sysrq_irqrestore(port, flags);
> @@ -1015,7 +1002,7 @@ static int stm32_usart_startup(struct uart_port *port)
>   	u32 val;
>   	int ret;
>   
> -	ret = request_threaded_irq(port->irq, stm32_usart_interrupt,
> +	ret = request_threaded_irq(port->irq, NULL,
>   				   stm32_usart_threaded_interrupt,
>   				   IRQF_ONESHOT | IRQF_NO_SUSPEND,
>   				   name, port);