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

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

[Breno Leitao]

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@linutronix.de>
Link: https://lore.kernel.org/all/87wlqtxb14.fsf@jogness.linutronix.de/ [1]
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/tty/serial/amba-pl011.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index e6f652905ca6f1..4fac3bca8f8255 100644
--- 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();
+
 	if (!nbcon_enter_unsafe(wctxt))
 		return;
 

---
base-commit: d6bcfc94eb4da6e089c5473970cd96727292fd97
base-tree: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
change-id: 20261104-pl011_touch-d01385c0a5d7

Best regards,
--  
Breno Leitao <leitao@debian.org>

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

[sashiko-bot]

> 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@linutronix.de>
> Link: https://lore.kernel.org/all/87wlqtxb14.fsf@jogness.linutronix.de/ [1]
> Signed-off-by: Breno Leitao <leitao@debian.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-pl011_touch-v1-1-e292bd834e14@debian.org?part=1

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

[John Ogness]

On 2026-11-04, Breno Leitao <leitao@debian.org> 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@linutronix.de>
> Link: https://lore.kernel.org/all/87wlqtxb14.fsf@jogness.linutronix.de/ [1]
> Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: John Ogness <john.ogness@linutronix.de>

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@linutronix.de>
> Link: https://lore.kernel.org/all/87wlqtxb14.fsf@jogness.linutronix.de/ [1]
> Signed-off-by: Breno Leitao <leitao@debian.org>

Makes sense:

Reviewed-by: Petr Mladek <pmladek@suse.com>

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

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

[Breno Leitao]

On Thu, Nov 05, 2026 at 11:37:39AM +0100, Petr Mladek wrote:
> > --- 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.

Ack, let me move it as you've just suggested, given I have no preference
at all. I will respin it soon.

Thanks for the review,
--breno