| Message ID | 20250110213822.2107462-1-sean.anderson@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tty: xilinx_uartps: split sysrq handling | expand |
On 2025-01-10, Sean Anderson <sean.anderson@linux.dev> wrote: > Fix this by splitting sysrq handling into two parts. We use the prepare > helper under the port lock and defer handling until we release the lock. Note that this fix is only necessary because this console driver is using the legacy console API. For the NBCON API it is allowed to call printk() while holding the port lock. But since code already exists to allow deferring the sysrq execution until the port lock is not held, this patch is probably a good idea anyway because it can reduce port lock contention. AFAIK there are no sysrq actions that require port lock synchronization. Acked-by: John Ogness <john.ogness@linutronix.de>
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index beb151be4d32..92ec51870d1d 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -287,7 +287,7 @@ static void cdns_uart_handle_rx(void *dev_id, unsigned int isrstatus) continue; } - if (uart_handle_sysrq_char(port, data)) + if (uart_prepare_sysrq_char(port, data)) continue; if (is_rxbs_support) { @@ -495,7 +495,7 @@ static irqreturn_t cdns_uart_isr(int irq, void *dev_id) !(readl(port->membase + CDNS_UART_CR) & CDNS_UART_CR_RX_DIS)) cdns_uart_handle_rx(dev_id, isrstatus); - uart_port_unlock(port); + uart_unlock_and_check_sysrq(port); return IRQ_HANDLED; } @@ -1380,9 +1380,7 @@ static void cdns_uart_console_write(struct console *co, const char *s, unsigned int imr, ctrl; int locked = 1; - if (port->sysrq) - locked = 0; - else if (oops_in_progress) + if (oops_in_progress) locked = uart_port_trylock_irqsave(port, &flags); else uart_port_lock_irqsave(port, &flags);
lockdep detects the following circular locking dependency: CPU 0 CPU 1 ========================== ============================ cdns_uart_isr() printk() uart_port_lock(port) console_lock() cdns_uart_console_write() if (!port->sysrq) uart_port_lock(port) uart_handle_break() port->sysrq = ... uart_handle_sysrq_char() printk() console_lock() The fixed commit attempts to avoid this situation by only taking the port lock in cdns_uart_console_write if port->sysrq unset. However, if (as shown above) cdns_uart_console_write runs before port->sysrq is set, then it will try to take the port lock anyway. This may result in a deadlock. Fix this by splitting sysrq handling into two parts. We use the prepare helper under the port lock and defer handling until we release the lock. Fixes: 74ea66d4ca06 ("tty: xuartps: Improve sysrq handling") Signed-off-by: Sean Anderson <sean.anderson@linux.dev> Cc: <stable@vger.kernel.org> # c980248179d: serial: xilinx_uartps: Use port lock wrappers --- drivers/tty/serial/xilinx_uartps.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)