Re: [PATCH] serial: amba-pl011: touch the watchdogs in write_atomic()

[Petr Mladek]

On Wed 2026-11-04 02:35:51, Breno Leitao wrote:
> In emergency context, such as WARN(), nbcon prints the pending records
> through write_atomic() with preemption disabled.
> 
> On arm64, WARN()  can run with IRQs masked, so the whole backlog is
> printed with IRQs off as well.
> 
> Commit 9bd18e1262c0 ("printk/nbcon: Restore IRQ in atomic flush after
> each emitted record") does not help here, as it only restores the
> caller's IRQ state between records.
> 
> pl011_console_write_atomic() busy-waits on the UART for every
> character, and at 115200 baud it prints about 11.5KB per second.
> Flushing a few hundred kilobytes outlasts the hard lockup threshold
> (10 seconds by default), so the printing CPU is reported, or the
> machine panics with hardlockup_panic set. See [1].
> 
> The 8250 driver avoids this by calling touch_nmi_watchdog() while it
> polls the transmitter. Do the same in pl011_console_write_atomic(),
> once per record.
> 
> Suggested-by: John Ogness <john.ogness@xxxxxxxxxxxxx>
> Link: https://lore.kernel.org/all/87wlqtxb14.fsf@xxxxxxxxxxxxxxxxxxxxx/ [1]
> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>

Makes sense:

Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>

See a comment below.

> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -41,6 +41,7 @@
>  #include <linux/sizes.h>
>  #include <linux/io.h>
>  #include <linux/acpi.h>
> +#include <linux/nmi.h>
>  
>  #define UART_NR			14
>  
> @@ -2734,6 +2735,9 @@ pl011_console_write_atomic(struct console *co, struct nbcon_write_context *wctxt
>  	struct uart_amba_port *uap = amba_ports[co->index];
>  	unsigned int old_cr = 0;
>  
> +	/* An atomic flush may print a long backlog with IRQs disabled. */
> +	touch_nmi_watchdog();
> +

I would personally touch the watchdog at the end before returning from
the function because it still could spend some non-trivial time
in this function.

But I do not have strong opinion about it. AFAIK, there is no official
recommendation. Some callers do it before and some after the delay.

And it should not make a real-life difference because a single
pl011_console_write_atomic() should not trigger the watchdog report.
The problem is when it is called many times in an atomic context.

So that it probably does not matter after all. It is a matter
of taste...

Feel free to keep it as is.

>  	if (!nbcon_enter_unsafe(wctxt))
>  		return;

Best Regards,
Petr