linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush
@ 2025-09-26 12:49 Petr Mladek
  2025-09-26 12:49 ` [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context Petr Mladek
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Petr Mladek @ 2025-09-26 12:49 UTC (permalink / raw)
  To: John Ogness
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray, Petr Mladek

This patchset should solve problem which was being discussed
at https://lore.kernel.org/all/aNFR45fL2L4PavNc@pathway.suse.cz

__nbcon_atomic_flush_pending_con() preserves the nbcon console
ownership all the time when flushing pending messages. It might
take a long time with slow serial consoles.

It might trigger a hardlockup report on another CPU which is
busy waiting for the nbcon console ownership, for example,
in nbcon_reacquire_nobuf() or __uart_port_nbcon_acquire().

The problem is solved by the 3rd patch. It releases the console
context ownership after each record.

The 3rd patch alone would increase the risk of takeovers and repeated
lines. It is prevented by the 1st patch which blocks the printk kthread
when any CPU is in an emergency context.

The 2nd patch allows to block the printk kthread also in panic.
It is not important. It is just an obvious update of the check
for emergency contexts.

Note: The patchset applies against current Linus' tree (v6.17-rc7).

      The 2nd patch would need an update after the consolisation of
      the panic state API gets merged via -mm tree,
      see https://lore.kernel.org/r/20250825022947.1596226-2-wangjinchao600@gmail.com

Petr Mladek (3):
  printk/nbcon: Block printk kthreads when any CPU is in an emergency
    context
  printk/nbcon/panic: Allow printk kthread to sleep when the system is
    in panic
  printk/nbcon: Release nbcon consoles ownership in atomic flush after
    each emitted record

 kernel/printk/internal.h |  1 +
 kernel/printk/nbcon.c    | 43 +++++++++++++++++++++++++++++++++++-----
 kernel/printk/printk.c   |  2 +-
 3 files changed, 40 insertions(+), 6 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context
  2025-09-26 12:49 [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush Petr Mladek
@ 2025-09-26 12:49 ` Petr Mladek
  2025-09-26 14:37   ` John Ogness
                     ` (2 more replies)
  2025-09-26 12:49 ` [PATCH 2/3] printk/nbcon/panic: Allow printk kthread to sleep when the system is in panic Petr Mladek
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 13+ messages in thread
From: Petr Mladek @ 2025-09-26 12:49 UTC (permalink / raw)
  To: John Ogness
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray, Petr Mladek

In emergency contexts, printk() tries to flush messages directly even
on nbcon consoles. And it is allowed to takeover the console ownership
and interrupt the printk kthread in the middle of a message.

Only one takeover and one repeated message should be enough in most
situations. The first emergency message flushes the backlog and printk
kthreads get to sleep. Next emergency messages are flushed directly
and printk() does not wake up the kthreads.

However, the one takeover is not guaranteed. Any printk() in normal
context on another CPU could wake up the kthreads. Or a new emergency
message might be added before the kthreads get to sleep. Note that
the interrupted .write_kthread() callbacks usually have to call
nbcon_reacquire_nobuf() and restore the original device setting
before checking for pending messages.

The risk of the repeated takeovers will be even bigger because
__nbcon_atomic_flush_pending_con is going to release the console
ownership after each emitted record. It will be needed to prevent
hardlockup reports on other CPUs which are busy waiting for
the context ownership, for example, by nbcon_reacquire_nobuf() or
__uart_port_nbcon_acquire().

The repeated takeovers break the output, for example:

    [ 5042.650211][ T2220] Call Trace:
    [ 5042.6511
    ** replaying previous printk message **
    [ 5042.651192][ T2220]  <TASK>
    [ 5042.652160][ T2220]  kunit_run_
    ** replaying previous printk message **
    [ 5042.652160][ T2220]  kunit_run_tests+0x72/0x90
    [ 5042.653340][ T22
    ** replaying previous printk message **
    [ 5042.653340][ T2220]  ? srso_alias_return_thunk+0x5/0xfbef5
    [ 5042.654628][ T2220]  ? stack_trace_save+0x4d/0x70
    [ 5042.6553
    ** replaying previous printk message **
    [ 5042.655394][ T2220]  ? srso_alias_return_thunk+0x5/0xfbef5
    [ 5042.656713][ T2220]  ? save_trace+0x5b/0x180

A more robust solution is to block the printk kthread entirely whenever
*any* CPU enters an emergency context. This ensures that critical messages
can be flushed without contention from the normal, non-atomic printing
path.

Link: https://lore.kernel.org/all/aNQO-zl3k1l4ENfy@pathway.suse.cz
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/nbcon.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index d5d8c8c657e0..08b196e898cd 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -117,6 +117,9 @@
  * from scratch.
  */
 
+/* Counter of active nbcon emergency contexts. */
+atomic_t nbcon_cpu_emergency_cnt;
+
 /**
  * nbcon_state_set - Helper function to set the console state
  * @con:	Console to update
@@ -1168,6 +1171,16 @@ static bool nbcon_kthread_should_wakeup(struct console *con, struct nbcon_contex
 	if (kthread_should_stop())
 		return true;
 
+	/*
+	 * Block the kthread when the system is in an emergency or panic mode.
+	 * It increases the chance that these contexts would be able to show
+	 * the messages directly. And it reduces the risk of interrupted writes
+	 * where the context with a higher priority takes over the nbcon console
+	 * ownership in the middle of a message.
+	 */
+	if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
+		return false;
+
 	cookie = console_srcu_read_lock();
 
 	flags = console_srcu_read_flags(con);
@@ -1219,6 +1232,13 @@ static int nbcon_kthread_func(void *__console)
 		if (kthread_should_stop())
 			return 0;
 
+		/*
+		 * Block the kthread when the system is in an emergency or panic
+		 * mode. See nbcon_kthread_should_wakeup() for more details.
+		 */
+		if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
+			goto wait_for_event;
+
 		backlog = false;
 
 		/*
@@ -1660,6 +1680,8 @@ void nbcon_cpu_emergency_enter(void)
 
 	preempt_disable();
 
+	atomic_inc(&nbcon_cpu_emergency_cnt);
+
 	cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
 	(*cpu_emergency_nesting)++;
 }
@@ -1674,10 +1696,18 @@ void nbcon_cpu_emergency_exit(void)
 	unsigned int *cpu_emergency_nesting;
 
 	cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
-
 	if (!WARN_ON_ONCE(*cpu_emergency_nesting == 0))
 		(*cpu_emergency_nesting)--;
 
+	/*
+	 * Wake up kthreads because there might be some pending messages
+	 * added by other CPUs with normal priority since the last flush
+	 * in the emergency context.
+	 */
+	if (!WARN_ON_ONCE(atomic_read(&nbcon_cpu_emergency_cnt) == 0))
+		if (atomic_dec_return(&nbcon_cpu_emergency_cnt) == 0)
+			nbcon_kthreads_wake();
+
 	preempt_enable();
 }
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] printk/nbcon/panic: Allow printk kthread to sleep when the system is in panic
  2025-09-26 12:49 [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush Petr Mladek
  2025-09-26 12:49 ` [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context Petr Mladek
@ 2025-09-26 12:49 ` Petr Mladek
  2025-09-26 14:38   ` John Ogness
  2025-09-29  8:39   ` Andrew Murray
  2025-09-26 12:49 ` [PATCH 3/3] printk/nbcon: Release nbcon consoles ownership in atomic flush after each emitted record Petr Mladek
  2025-10-30 11:32 ` [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush Petr Mladek
  3 siblings, 2 replies; 13+ messages in thread
From: Petr Mladek @ 2025-09-26 12:49 UTC (permalink / raw)
  To: John Ogness
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray, Petr Mladek

The printk kthread might be running when there is a panic in progress.
But it is not able to acquire the console ownership any longer.

Prevent the desperate attempts to acquire the ownership and allow sleeping
in panic. It would make it behave the same as when there is any CPU
in an emergency context.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/internal.h | 1 +
 kernel/printk/nbcon.c    | 6 ++++--
 kernel/printk/printk.c   | 2 +-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index ef282001f200..6e8578102fb3 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -332,6 +332,7 @@ struct printk_message {
 	unsigned long		dropped;
 };
 
+bool panic_in_progress(void);
 bool other_cpu_in_panic(void);
 bool printk_get_next_message(struct printk_message *pmsg, u64 seq,
 			     bool is_extended, bool may_supress);
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 08b196e898cd..219ae0c8b5ed 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1178,7 +1178,8 @@ static bool nbcon_kthread_should_wakeup(struct console *con, struct nbcon_contex
 	 * where the context with a higher priority takes over the nbcon console
 	 * ownership in the middle of a message.
 	 */
-	if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
+	if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)) ||
+	    unlikely(panic_in_progress()))
 		return false;
 
 	cookie = console_srcu_read_lock();
@@ -1236,7 +1237,8 @@ static int nbcon_kthread_func(void *__console)
 		 * Block the kthread when the system is in an emergency or panic
 		 * mode. See nbcon_kthread_should_wakeup() for more details.
 		 */
-		if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
+		if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)) ||
+		    unlikely(panic_in_progress()))
 			goto wait_for_event;
 
 		backlog = false;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index ebf10352736f..174d42041594 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -345,7 +345,7 @@ static void __up_console_sem(unsigned long ip)
 }
 #define up_console_sem() __up_console_sem(_RET_IP_)
 
-static bool panic_in_progress(void)
+bool panic_in_progress(void)
 {
 	return unlikely(atomic_read(&panic_cpu) != PANIC_CPU_INVALID);
 }
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] printk/nbcon: Release nbcon consoles ownership in atomic flush after each emitted record
  2025-09-26 12:49 [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush Petr Mladek
  2025-09-26 12:49 ` [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context Petr Mladek
  2025-09-26 12:49 ` [PATCH 2/3] printk/nbcon/panic: Allow printk kthread to sleep when the system is in panic Petr Mladek
@ 2025-09-26 12:49 ` Petr Mladek
  2025-09-26 14:43   ` John Ogness
  2025-09-29  8:38   ` Andrew Murray
  2025-10-30 11:32 ` [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush Petr Mladek
  3 siblings, 2 replies; 13+ messages in thread
From: Petr Mladek @ 2025-09-26 12:49 UTC (permalink / raw)
  To: John Ogness
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray, Petr Mladek

printk() tries to flush messages with NBCON_PRIO_EMERGENCY on
nbcon consoles immediately. It might take seconds to flush all
pending lines on slow serial consoles. Note that there might be
hundreds of messages, for example:

[    3.771531][    T1] pci 0000:3e:08.1: [8086:324
** replaying previous printk message **
[    3.771531][    T1] pci 0000:3e:08.1: [8086:3246] type 00 class 0x088000 PCIe Root Complex Integrated Endpoint
[ ... more than 2000 lines, about 200kB messages ... ]
[    3.837752][    T1] pci 0000:20:01.0: Adding to iommu group 18
[    3.837851][    T
** replaying previous printk message **
[    3.837851][    T1] pci 0000:20:03.0: Adding to iommu group 19
[    3.837946][    T1] pci 0000:20:05.0: Adding to iommu group 20
[ ... more than 500 messages for iommu groups 21-590 ...]
[    3.912932][    T1] pci 0000:f6:00.1: Adding to iommu group 591
[    3.913070][    T1] pci 0000:f6:00.2: Adding to iommu group 592
[    3.913243][    T1] DMAR: Intel(R) Virtualization Technology for Directed I/O
[    3.913245][    T1] PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
[    3.913245][    T1] software IO TLB: mapped [mem 0x000000004f000000-0x0000000053000000] (64MB)
[    3.913324][    T1] RAPL PMU: API unit is 2^-32 Joules, 3 fixed counters, 655360 ms ovfl timer
[    3.913325][    T1] RAPL PMU: hw unit of domain package 2^-14 Joules
[    3.913326][    T1] RAPL PMU: hw unit of domain dram 2^-14 Joules
[    3.913327][    T1] RAPL PMU: hw unit of domain psys 2^-0 Joules
[    3.933486][    T1] ------------[ cut here ]------------
[    3.933488][    T1] WARNING: CPU: 2 PID: 1 at arch/x86/events/intel/uncore.c:1156 uncore_pci_pmu_register+0x15e/0x180
[    3.930291][    C0] watchdog: Watchdog detected hard LOCKUP on cpu 0
[    3.930291][    C0] Kernel panic - not syncing: Hard LOCKUP
[...]
[    3.930291][    C0] CPU: 0 UID: 0 PID: 18 Comm: pr/ttyS0 Not tainted...
[...]
[    3.930291][    C0] RIP: 0010:nbcon_reacquire_nobuf+0x11/0x50
[    3.930291][    C0] Call Trace:
[...]
[    3.930291][    C0]  <TASK>
[    3.930291][    C0]  serial8250_console_write+0x16d/0x5c0
[    3.930291][    C0]  nbcon_emit_next_record+0x22c/0x250
[    3.930291][    C0]  nbcon_emit_one+0x93/0xe0
[    3.930291][    C0]  nbcon_kthread_func+0x13c/0x1c0

The are visible two takeovers of the console ownership:

  - The 1st one is triggered by the "WARNING: CPU: 2 PID: 1 at
    arch/x86/..." line printed with NBCON_PRIO_EMERGENCY.

  - The 2nd one is triggered by the "Kernel panic - not syncing:
    Hard LOCKUP" line printed with NBCON_PRIO_PANIC.

There are more than 2500 lines, at about 240kB, emitted between
the takeover and the 1st "WARNING" line in the emergency context.
This amount of pending messages had to be flushed by
nbcon_atomic_flush_pending() when WARN() printed its first line.

The atomic flush was holding the nbcon console context for too long so
that it triggered hard lockup on the CPU running the printk kthread
"pr/ttyS0". The kthread needed to reacquire the console ownership
for restoring the original serial port state in serial8250_console_write().

Prevent the hardlockup by releasing the nbcon console ownership after
each emitted record.

Note that __nbcon_atomic_flush_pending_con() used to hold the console
ownership all the time because it blocked the printk kthread. Otherwise
the kthread tried to flush the messages in parallel which caused repeated
takeovers and more replayed messages.

It is not longer a problem because the repeated takeovers are blocked
by the counter of emergency contexts, see nbcon_cpu_emergency_cnt.

Link: https://lore.kernel.org/all/aNQO-zl3k1l4ENfy@pathway.suse.cz
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/nbcon.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 219ae0c8b5ed..e298346111b2 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1532,10 +1532,10 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
 	ctxt->prio			= nbcon_get_default_prio();
 	ctxt->allow_unsafe_takeover	= allow_unsafe_takeover;
 
-	if (!nbcon_context_try_acquire(ctxt, false))
-		return -EPERM;
-
 	while (nbcon_seq_read(con) < stop_seq) {
+		if (!nbcon_context_try_acquire(ctxt, false))
+			return -EPERM;
+
 		/*
 		 * nbcon_emit_next_record() returns false when the console was
 		 * handed over or taken over. In both cases the context is no
@@ -1544,6 +1544,8 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
 		if (!nbcon_emit_next_record(&wctxt, true))
 			return -EAGAIN;
 
+		nbcon_context_release(ctxt);
+
 		if (!ctxt->backlog) {
 			/* Are there reserved but not yet finalized records? */
 			if (nbcon_seq_read(con) < stop_seq)
@@ -1552,7 +1554,6 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
 		}
 	}
 
-	nbcon_context_release(ctxt);
 	return err;
 }
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context
  2025-09-26 12:49 ` [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context Petr Mladek
@ 2025-09-26 14:37   ` John Ogness
  2025-09-29 12:02     ` Petr Mladek
  2025-09-29  8:40   ` Andrew Murray
  2025-09-30 20:15   ` kernel test robot
  2 siblings, 1 reply; 13+ messages in thread
From: John Ogness @ 2025-09-26 14:37 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray, Petr Mladek

On 2025-09-26, Petr Mladek <pmladek@suse.com> wrote:
> In emergency contexts, printk() tries to flush messages directly even
> on nbcon consoles. And it is allowed to takeover the console ownership
> and interrupt the printk kthread in the middle of a message.
>
> Only one takeover and one repeated message should be enough in most
> situations. The first emergency message flushes the backlog and printk
> kthreads get to sleep. Next emergency messages are flushed directly
> and printk() does not wake up the kthreads.
>
> However, the one takeover is not guaranteed. Any printk() in normal
> context on another CPU could wake up the kthreads. Or a new emergency
> message might be added before the kthreads get to sleep. Note that
> the interrupted .write_kthread() callbacks usually have to call

                  .write_thread()

> nbcon_reacquire_nobuf() and restore the original device setting
> before checking for pending messages.
>
> The risk of the repeated takeovers will be even bigger because
> __nbcon_atomic_flush_pending_con is going to release the console
> ownership after each emitted record. It will be needed to prevent
> hardlockup reports on other CPUs which are busy waiting for
> the context ownership, for example, by nbcon_reacquire_nobuf() or
> __uart_port_nbcon_acquire().
>
> The repeated takeovers break the output, for example:
>
>     [ 5042.650211][ T2220] Call Trace:
>     [ 5042.6511
>     ** replaying previous printk message **
>     [ 5042.651192][ T2220]  <TASK>
>     [ 5042.652160][ T2220]  kunit_run_
>     ** replaying previous printk message **
>     [ 5042.652160][ T2220]  kunit_run_tests+0x72/0x90
>     [ 5042.653340][ T22
>     ** replaying previous printk message **
>     [ 5042.653340][ T2220]  ? srso_alias_return_thunk+0x5/0xfbef5
>     [ 5042.654628][ T2220]  ? stack_trace_save+0x4d/0x70
>     [ 5042.6553
>     ** replaying previous printk message **
>     [ 5042.655394][ T2220]  ? srso_alias_return_thunk+0x5/0xfbef5
>     [ 5042.656713][ T2220]  ? save_trace+0x5b/0x180
>
> A more robust solution is to block the printk kthread entirely whenever
> *any* CPU enters an emergency context. This ensures that critical messages
> can be flushed without contention from the normal, non-atomic printing
> path.
>
> Link: https://lore.kernel.org/all/aNQO-zl3k1l4ENfy@pathway.suse.cz
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  kernel/printk/nbcon.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index d5d8c8c657e0..08b196e898cd 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -117,6 +117,9 @@
>   * from scratch.
>   */
>  
> +/* Counter of active nbcon emergency contexts. */
> +atomic_t nbcon_cpu_emergency_cnt;

This can be static and should be initialized:

static atomic_t nbcon_cpu_emergency_cnt = ATOMIC_INIT(0);

> +
>  /**
>   * nbcon_state_set - Helper function to set the console state
>   * @con:	Console to update
> @@ -1168,6 +1171,16 @@ static bool nbcon_kthread_should_wakeup(struct console *con, struct nbcon_contex
>  	if (kthread_should_stop())
>  		return true;
>  
> +	/*
> +	 * Block the kthread when the system is in an emergency or panic mode.
> +	 * It increases the chance that these contexts would be able to show
> +	 * the messages directly. And it reduces the risk of interrupted writes
> +	 * where the context with a higher priority takes over the nbcon console
> +	 * ownership in the middle of a message.
> +	 */
> +	if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
> +		return false;
> +
>  	cookie = console_srcu_read_lock();
>  
>  	flags = console_srcu_read_flags(con);
> @@ -1219,6 +1232,13 @@ static int nbcon_kthread_func(void *__console)
>  		if (kthread_should_stop())
>  			return 0;
>  
> +		/*
> +		 * Block the kthread when the system is in an emergency or panic
> +		 * mode. See nbcon_kthread_should_wakeup() for more details.
> +		 */
> +		if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
> +			goto wait_for_event;
> +
>  		backlog = false;
>  
>  		/*
> @@ -1660,6 +1680,8 @@ void nbcon_cpu_emergency_enter(void)
>  
>  	preempt_disable();
>  
> +	atomic_inc(&nbcon_cpu_emergency_cnt);
> +
>  	cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
>  	(*cpu_emergency_nesting)++;
>  }
> @@ -1674,10 +1696,18 @@ void nbcon_cpu_emergency_exit(void)
>  	unsigned int *cpu_emergency_nesting;
>  
>  	cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
> -
>  	if (!WARN_ON_ONCE(*cpu_emergency_nesting == 0))
>  		(*cpu_emergency_nesting)--;
>  
> +	/*
> +	 * Wake up kthreads because there might be some pending messages
> +	 * added by other CPUs with normal priority since the last flush
> +	 * in the emergency context.
> +	 */
> +	if (!WARN_ON_ONCE(atomic_read(&nbcon_cpu_emergency_cnt) == 0))
> +		if (atomic_dec_return(&nbcon_cpu_emergency_cnt) == 0)
> +			nbcon_kthreads_wake();

Although technically it doesn't hurt to blindly call
nbcon_kthreads_wake(), you may want to do it more formally. Maybe like
this:

	if (!WARN_ON_ONCE(atomic_read(&nbcon_cpu_emergency_cnt) == 0)) {
		if (atomic_dec_return(&nbcon_cpu_emergency_cnt) == 0) {
			struct console_flush_type ft;

			printk_get_console_flush_type(&ft);
			if (ft.nbcon_offload)
				nbcon_kthreads_wake();
		}
	}

I leave it up to you.

With the static+initializer change:

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] printk/nbcon/panic: Allow printk kthread to sleep when the system is in panic
  2025-09-26 12:49 ` [PATCH 2/3] printk/nbcon/panic: Allow printk kthread to sleep when the system is in panic Petr Mladek
@ 2025-09-26 14:38   ` John Ogness
  2025-09-29  8:39   ` Andrew Murray
  1 sibling, 0 replies; 13+ messages in thread
From: John Ogness @ 2025-09-26 14:38 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray, Petr Mladek

On 2025-09-26, Petr Mladek <pmladek@suse.com> wrote:
> The printk kthread might be running when there is a panic in progress.
> But it is not able to acquire the console ownership any longer.
>
> Prevent the desperate attempts to acquire the ownership and allow sleeping
> in panic. It would make it behave the same as when there is any CPU
> in an emergency context.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] printk/nbcon: Release nbcon consoles ownership in atomic flush after each emitted record
  2025-09-26 12:49 ` [PATCH 3/3] printk/nbcon: Release nbcon consoles ownership in atomic flush after each emitted record Petr Mladek
@ 2025-09-26 14:43   ` John Ogness
  2025-09-29  8:38   ` Andrew Murray
  1 sibling, 0 replies; 13+ messages in thread
From: John Ogness @ 2025-09-26 14:43 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray, Petr Mladek

On 2025-09-26, Petr Mladek <pmladek@suse.com> wrote:
> printk() tries to flush messages with NBCON_PRIO_EMERGENCY on
> nbcon consoles immediately. It might take seconds to flush all
> pending lines on slow serial consoles. Note that there might be
> hundreds of messages, for example:
>
> [    3.771531][    T1] pci 0000:3e:08.1: [8086:324
> ** replaying previous printk message **
> [    3.771531][    T1] pci 0000:3e:08.1: [8086:3246] type 00 class 0x088000 PCIe Root Complex Integrated Endpoint
> [ ... more than 2000 lines, about 200kB messages ... ]
> [    3.837752][    T1] pci 0000:20:01.0: Adding to iommu group 18
> [    3.837851][    T
> ** replaying previous printk message **
> [    3.837851][    T1] pci 0000:20:03.0: Adding to iommu group 19
> [    3.837946][    T1] pci 0000:20:05.0: Adding to iommu group 20
> [ ... more than 500 messages for iommu groups 21-590 ...]
> [    3.912932][    T1] pci 0000:f6:00.1: Adding to iommu group 591
> [    3.913070][    T1] pci 0000:f6:00.2: Adding to iommu group 592
> [    3.913243][    T1] DMAR: Intel(R) Virtualization Technology for Directed I/O
> [    3.913245][    T1] PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
> [    3.913245][    T1] software IO TLB: mapped [mem 0x000000004f000000-0x0000000053000000] (64MB)
> [    3.913324][    T1] RAPL PMU: API unit is 2^-32 Joules, 3 fixed counters, 655360 ms ovfl timer
> [    3.913325][    T1] RAPL PMU: hw unit of domain package 2^-14 Joules
> [    3.913326][    T1] RAPL PMU: hw unit of domain dram 2^-14 Joules
> [    3.913327][    T1] RAPL PMU: hw unit of domain psys 2^-0 Joules
> [    3.933486][    T1] ------------[ cut here ]------------
> [    3.933488][    T1] WARNING: CPU: 2 PID: 1 at arch/x86/events/intel/uncore.c:1156 uncore_pci_pmu_register+0x15e/0x180
> [    3.930291][    C0] watchdog: Watchdog detected hard LOCKUP on cpu 0
> [    3.930291][    C0] Kernel panic - not syncing: Hard LOCKUP
> [...]
> [    3.930291][    C0] CPU: 0 UID: 0 PID: 18 Comm: pr/ttyS0 Not tainted...
> [...]
> [    3.930291][    C0] RIP: 0010:nbcon_reacquire_nobuf+0x11/0x50
> [    3.930291][    C0] Call Trace:
> [...]
> [    3.930291][    C0]  <TASK>
> [    3.930291][    C0]  serial8250_console_write+0x16d/0x5c0
> [    3.930291][    C0]  nbcon_emit_next_record+0x22c/0x250
> [    3.930291][    C0]  nbcon_emit_one+0x93/0xe0
> [    3.930291][    C0]  nbcon_kthread_func+0x13c/0x1c0
>
> The are visible two takeovers of the console ownership:
>
>   - The 1st one is triggered by the "WARNING: CPU: 2 PID: 1 at
>     arch/x86/..." line printed with NBCON_PRIO_EMERGENCY.
>
>   - The 2nd one is triggered by the "Kernel panic - not syncing:
>     Hard LOCKUP" line printed with NBCON_PRIO_PANIC.
>
> There are more than 2500 lines, at about 240kB, emitted between
> the takeover and the 1st "WARNING" line in the emergency context.
> This amount of pending messages had to be flushed by
> nbcon_atomic_flush_pending() when WARN() printed its first line.
>
> The atomic flush was holding the nbcon console context for too long so
> that it triggered hard lockup on the CPU running the printk kthread
> "pr/ttyS0". The kthread needed to reacquire the console ownership
> for restoring the original serial port state in serial8250_console_write().
>
> Prevent the hardlockup by releasing the nbcon console ownership after
> each emitted record.
>
> Note that __nbcon_atomic_flush_pending_con() used to hold the console
> ownership all the time because it blocked the printk kthread. Otherwise
> the kthread tried to flush the messages in parallel which caused repeated
> takeovers and more replayed messages.
>
> It is not longer a problem because the repeated takeovers are blocked
> by the counter of emergency contexts, see nbcon_cpu_emergency_cnt.
>
> Link: https://lore.kernel.org/all/aNQO-zl3k1l4ENfy@pathway.suse.cz
> Signed-off-by: Petr Mladek <pmladek@suse.com>

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

Looks good and performs as advertised.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] printk/nbcon: Release nbcon consoles ownership in atomic flush after each emitted record
  2025-09-26 12:49 ` [PATCH 3/3] printk/nbcon: Release nbcon consoles ownership in atomic flush after each emitted record Petr Mladek
  2025-09-26 14:43   ` John Ogness
@ 2025-09-29  8:38   ` Andrew Murray
  1 sibling, 0 replies; 13+ messages in thread
From: Andrew Murray @ 2025-09-29  8:38 UTC (permalink / raw)
  To: Petr Mladek
  Cc: John Ogness, Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin

On Fri, 26 Sept 2025 at 13:50, Petr Mladek <pmladek@suse.com> wrote:
>
> printk() tries to flush messages with NBCON_PRIO_EMERGENCY on
> nbcon consoles immediately. It might take seconds to flush all
> pending lines on slow serial consoles. Note that there might be
> hundreds of messages, for example:
>
> [    3.771531][    T1] pci 0000:3e:08.1: [8086:324
> ** replaying previous printk message **
> [    3.771531][    T1] pci 0000:3e:08.1: [8086:3246] type 00 class 0x088000 PCIe Root Complex Integrated Endpoint
> [ ... more than 2000 lines, about 200kB messages ... ]
> [    3.837752][    T1] pci 0000:20:01.0: Adding to iommu group 18
> [    3.837851][    T
> ** replaying previous printk message **
> [    3.837851][    T1] pci 0000:20:03.0: Adding to iommu group 19
> [    3.837946][    T1] pci 0000:20:05.0: Adding to iommu group 20
> [ ... more than 500 messages for iommu groups 21-590 ...]
> [    3.912932][    T1] pci 0000:f6:00.1: Adding to iommu group 591
> [    3.913070][    T1] pci 0000:f6:00.2: Adding to iommu group 592
> [    3.913243][    T1] DMAR: Intel(R) Virtualization Technology for Directed I/O
> [    3.913245][    T1] PCI-DMA: Using software bounce buffering for IO (SWIOTLB)
> [    3.913245][    T1] software IO TLB: mapped [mem 0x000000004f000000-0x0000000053000000] (64MB)
> [    3.913324][    T1] RAPL PMU: API unit is 2^-32 Joules, 3 fixed counters, 655360 ms ovfl timer
> [    3.913325][    T1] RAPL PMU: hw unit of domain package 2^-14 Joules
> [    3.913326][    T1] RAPL PMU: hw unit of domain dram 2^-14 Joules
> [    3.913327][    T1] RAPL PMU: hw unit of domain psys 2^-0 Joules
> [    3.933486][    T1] ------------[ cut here ]------------
> [    3.933488][    T1] WARNING: CPU: 2 PID: 1 at arch/x86/events/intel/uncore.c:1156 uncore_pci_pmu_register+0x15e/0x180
> [    3.930291][    C0] watchdog: Watchdog detected hard LOCKUP on cpu 0
> [    3.930291][    C0] Kernel panic - not syncing: Hard LOCKUP
> [...]
> [    3.930291][    C0] CPU: 0 UID: 0 PID: 18 Comm: pr/ttyS0 Not tainted...
> [...]
> [    3.930291][    C0] RIP: 0010:nbcon_reacquire_nobuf+0x11/0x50
> [    3.930291][    C0] Call Trace:
> [...]
> [    3.930291][    C0]  <TASK>
> [    3.930291][    C0]  serial8250_console_write+0x16d/0x5c0
> [    3.930291][    C0]  nbcon_emit_next_record+0x22c/0x250
> [    3.930291][    C0]  nbcon_emit_one+0x93/0xe0
> [    3.930291][    C0]  nbcon_kthread_func+0x13c/0x1c0
>
> The are visible two takeovers of the console ownership:
>
>   - The 1st one is triggered by the "WARNING: CPU: 2 PID: 1 at
>     arch/x86/..." line printed with NBCON_PRIO_EMERGENCY.
>
>   - The 2nd one is triggered by the "Kernel panic - not syncing:
>     Hard LOCKUP" line printed with NBCON_PRIO_PANIC.
>
> There are more than 2500 lines, at about 240kB, emitted between
> the takeover and the 1st "WARNING" line in the emergency context.
> This amount of pending messages had to be flushed by
> nbcon_atomic_flush_pending() when WARN() printed its first line.
>
> The atomic flush was holding the nbcon console context for too long so
> that it triggered hard lockup on the CPU running the printk kthread
> "pr/ttyS0". The kthread needed to reacquire the console ownership
> for restoring the original serial port state in serial8250_console_write().
>
> Prevent the hardlockup by releasing the nbcon console ownership after
> each emitted record.
>
> Note that __nbcon_atomic_flush_pending_con() used to hold the console
> ownership all the time because it blocked the printk kthread. Otherwise
> the kthread tried to flush the messages in parallel which caused repeated
> takeovers and more replayed messages.
>
> It is not longer a problem because the repeated takeovers are blocked
> by the counter of emergency contexts, see nbcon_cpu_emergency_cnt.
>
> Link: https://lore.kernel.org/all/aNQO-zl3k1l4ENfy@pathway.suse.cz
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  kernel/printk/nbcon.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 219ae0c8b5ed..e298346111b2 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1532,10 +1532,10 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
>         ctxt->prio                      = nbcon_get_default_prio();
>         ctxt->allow_unsafe_takeover     = allow_unsafe_takeover;
>
> -       if (!nbcon_context_try_acquire(ctxt, false))
> -               return -EPERM;
> -
>         while (nbcon_seq_read(con) < stop_seq) {
> +               if (!nbcon_context_try_acquire(ctxt, false))
> +                       return -EPERM;
> +
>                 /*
>                  * nbcon_emit_next_record() returns false when the console was
>                  * handed over or taken over. In both cases the context is no
> @@ -1544,6 +1544,8 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
>                 if (!nbcon_emit_next_record(&wctxt, true))
>                         return -EAGAIN;
>
> +               nbcon_context_release(ctxt);
> +
>                 if (!ctxt->backlog) {
>                         /* Are there reserved but not yet finalized records? */
>                         if (nbcon_seq_read(con) < stop_seq)
> @@ -1552,7 +1554,6 @@ static int __nbcon_atomic_flush_pending_con(struct console *con, u64 stop_seq,
>                 }
>         }
>
> -       nbcon_context_release(ctxt);
>         return err;
>  }
>
> --
> 2.51.0
>

Reviewed-by: Andrew Murray <amurray@thegoodpenguin.co.uk>

Thanks,

Andrew Murray

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] printk/nbcon/panic: Allow printk kthread to sleep when the system is in panic
  2025-09-26 12:49 ` [PATCH 2/3] printk/nbcon/panic: Allow printk kthread to sleep when the system is in panic Petr Mladek
  2025-09-26 14:38   ` John Ogness
@ 2025-09-29  8:39   ` Andrew Murray
  1 sibling, 0 replies; 13+ messages in thread
From: Andrew Murray @ 2025-09-29  8:39 UTC (permalink / raw)
  To: Petr Mladek
  Cc: John Ogness, Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin

On Fri, 26 Sept 2025 at 13:50, Petr Mladek <pmladek@suse.com> wrote:
>
> The printk kthread might be running when there is a panic in progress.
> But it is not able to acquire the console ownership any longer.
>
> Prevent the desperate attempts to acquire the ownership and allow sleeping
> in panic. It would make it behave the same as when there is any CPU
> in an emergency context.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  kernel/printk/internal.h | 1 +
>  kernel/printk/nbcon.c    | 6 ++++--
>  kernel/printk/printk.c   | 2 +-
>  3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
> index ef282001f200..6e8578102fb3 100644
> --- a/kernel/printk/internal.h
> +++ b/kernel/printk/internal.h
> @@ -332,6 +332,7 @@ struct printk_message {
>         unsigned long           dropped;
>  };
>
> +bool panic_in_progress(void);
>  bool other_cpu_in_panic(void);
>  bool printk_get_next_message(struct printk_message *pmsg, u64 seq,
>                              bool is_extended, bool may_supress);
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index 08b196e898cd..219ae0c8b5ed 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -1178,7 +1178,8 @@ static bool nbcon_kthread_should_wakeup(struct console *con, struct nbcon_contex
>          * where the context with a higher priority takes over the nbcon console
>          * ownership in the middle of a message.
>          */
> -       if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
> +       if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)) ||
> +           unlikely(panic_in_progress()))
>                 return false;
>
>         cookie = console_srcu_read_lock();
> @@ -1236,7 +1237,8 @@ static int nbcon_kthread_func(void *__console)
>                  * Block the kthread when the system is in an emergency or panic
>                  * mode. See nbcon_kthread_should_wakeup() for more details.
>                  */
> -               if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
> +               if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)) ||
> +                   unlikely(panic_in_progress()))
>                         goto wait_for_event;
>
>                 backlog = false;
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index ebf10352736f..174d42041594 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -345,7 +345,7 @@ static void __up_console_sem(unsigned long ip)
>  }
>  #define up_console_sem() __up_console_sem(_RET_IP_)
>
> -static bool panic_in_progress(void)
> +bool panic_in_progress(void)
>  {
>         return unlikely(atomic_read(&panic_cpu) != PANIC_CPU_INVALID);
>  }
> --
> 2.51.0
>

Reviewed-by: Andrew Murray <amurray@thegoodpenguin.co.uk>

Thanks,

Andrew Murray

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context
  2025-09-26 12:49 ` [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context Petr Mladek
  2025-09-26 14:37   ` John Ogness
@ 2025-09-29  8:40   ` Andrew Murray
  2025-09-30 20:15   ` kernel test robot
  2 siblings, 0 replies; 13+ messages in thread
From: Andrew Murray @ 2025-09-29  8:40 UTC (permalink / raw)
  To: Petr Mladek
  Cc: John Ogness, Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin

On Fri, 26 Sept 2025 at 13:50, Petr Mladek <pmladek@suse.com> wrote:
>
> In emergency contexts, printk() tries to flush messages directly even
> on nbcon consoles. And it is allowed to takeover the console ownership
> and interrupt the printk kthread in the middle of a message.
>
> Only one takeover and one repeated message should be enough in most
> situations. The first emergency message flushes the backlog and printk
> kthreads get to sleep. Next emergency messages are flushed directly
> and printk() does not wake up the kthreads.
>
> However, the one takeover is not guaranteed. Any printk() in normal
> context on another CPU could wake up the kthreads. Or a new emergency
> message might be added before the kthreads get to sleep. Note that
> the interrupted .write_kthread() callbacks usually have to call
> nbcon_reacquire_nobuf() and restore the original device setting
> before checking for pending messages.
>
> The risk of the repeated takeovers will be even bigger because
> __nbcon_atomic_flush_pending_con is going to release the console
> ownership after each emitted record. It will be needed to prevent
> hardlockup reports on other CPUs which are busy waiting for
> the context ownership, for example, by nbcon_reacquire_nobuf() or
> __uart_port_nbcon_acquire().
>
> The repeated takeovers break the output, for example:
>
>     [ 5042.650211][ T2220] Call Trace:
>     [ 5042.6511
>     ** replaying previous printk message **
>     [ 5042.651192][ T2220]  <TASK>
>     [ 5042.652160][ T2220]  kunit_run_
>     ** replaying previous printk message **
>     [ 5042.652160][ T2220]  kunit_run_tests+0x72/0x90
>     [ 5042.653340][ T22
>     ** replaying previous printk message **
>     [ 5042.653340][ T2220]  ? srso_alias_return_thunk+0x5/0xfbef5
>     [ 5042.654628][ T2220]  ? stack_trace_save+0x4d/0x70
>     [ 5042.6553
>     ** replaying previous printk message **
>     [ 5042.655394][ T2220]  ? srso_alias_return_thunk+0x5/0xfbef5
>     [ 5042.656713][ T2220]  ? save_trace+0x5b/0x180
>
> A more robust solution is to block the printk kthread entirely whenever
> *any* CPU enters an emergency context. This ensures that critical messages
> can be flushed without contention from the normal, non-atomic printing
> path.
>
> Link: https://lore.kernel.org/all/aNQO-zl3k1l4ENfy@pathway.suse.cz
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> ---
>  kernel/printk/nbcon.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
> index d5d8c8c657e0..08b196e898cd 100644
> --- a/kernel/printk/nbcon.c
> +++ b/kernel/printk/nbcon.c
> @@ -117,6 +117,9 @@
>   * from scratch.
>   */
>
> +/* Counter of active nbcon emergency contexts. */
> +atomic_t nbcon_cpu_emergency_cnt;
> +
>  /**
>   * nbcon_state_set - Helper function to set the console state
>   * @con:       Console to update
> @@ -1168,6 +1171,16 @@ static bool nbcon_kthread_should_wakeup(struct console *con, struct nbcon_contex
>         if (kthread_should_stop())
>                 return true;
>
> +       /*
> +        * Block the kthread when the system is in an emergency or panic mode.
> +        * It increases the chance that these contexts would be able to show
> +        * the messages directly. And it reduces the risk of interrupted writes
> +        * where the context with a higher priority takes over the nbcon console
> +        * ownership in the middle of a message.
> +        */
> +       if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
> +               return false;
> +
>         cookie = console_srcu_read_lock();
>
>         flags = console_srcu_read_flags(con);
> @@ -1219,6 +1232,13 @@ static int nbcon_kthread_func(void *__console)
>                 if (kthread_should_stop())
>                         return 0;
>
> +               /*
> +                * Block the kthread when the system is in an emergency or panic
> +                * mode. See nbcon_kthread_should_wakeup() for more details.
> +                */
> +               if (unlikely(atomic_read(&nbcon_cpu_emergency_cnt)))
> +                       goto wait_for_event;
> +
>                 backlog = false;
>
>                 /*
> @@ -1660,6 +1680,8 @@ void nbcon_cpu_emergency_enter(void)
>
>         preempt_disable();
>
> +       atomic_inc(&nbcon_cpu_emergency_cnt);
> +
>         cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
>         (*cpu_emergency_nesting)++;
>  }
> @@ -1674,10 +1696,18 @@ void nbcon_cpu_emergency_exit(void)
>         unsigned int *cpu_emergency_nesting;
>
>         cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
> -
>         if (!WARN_ON_ONCE(*cpu_emergency_nesting == 0))
>                 (*cpu_emergency_nesting)--;
>
> +       /*
> +        * Wake up kthreads because there might be some pending messages
> +        * added by other CPUs with normal priority since the last flush
> +        * in the emergency context.
> +        */
> +       if (!WARN_ON_ONCE(atomic_read(&nbcon_cpu_emergency_cnt) == 0))
> +               if (atomic_dec_return(&nbcon_cpu_emergency_cnt) == 0)
> +                       nbcon_kthreads_wake();
> +
>         preempt_enable();
>  }
>
> --
> 2.51.0
>

Reviewed-by: Andrew Murray <amurray@thegoodpenguin.co.uk>

Thanks,

Andrew Murray

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context
  2025-09-26 14:37   ` John Ogness
@ 2025-09-29 12:02     ` Petr Mladek
  0 siblings, 0 replies; 13+ messages in thread
From: Petr Mladek @ 2025-09-29 12:02 UTC (permalink / raw)
  To: John Ogness
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray

On Fri 2025-09-26 16:43:33, John Ogness wrote:
> On 2025-09-26, Petr Mladek <pmladek@suse.com> wrote:
> > In emergency contexts, printk() tries to flush messages directly even
> > on nbcon consoles. And it is allowed to takeover the console ownership
> > and interrupt the printk kthread in the middle of a message.
> >
> > Only one takeover and one repeated message should be enough in most
> > situations. The first emergency message flushes the backlog and printk
> > kthreads get to sleep. Next emergency messages are flushed directly
> > and printk() does not wake up the kthreads.
> >
> > However, the one takeover is not guaranteed. Any printk() in normal
> > context on another CPU could wake up the kthreads. Or a new emergency
> > message might be added before the kthreads get to sleep. Note that
> > the interrupted .write_kthread() callbacks usually have to call
> 
>                   .write_thread()

Oh my muscle memory ;-)

> > nbcon_reacquire_nobuf() and restore the original device setting
> > before checking for pending messages.

[...]

> > --- a/kernel/printk/nbcon.c
> > +++ b/kernel/printk/nbcon.c
> > @@ -1674,10 +1696,18 @@ void nbcon_cpu_emergency_exit(void)
> >  	unsigned int *cpu_emergency_nesting;
> >  
> >  	cpu_emergency_nesting = nbcon_get_cpu_emergency_nesting();
> > -
> >  	if (!WARN_ON_ONCE(*cpu_emergency_nesting == 0))
> >  		(*cpu_emergency_nesting)--;
> >  
> > +	/*
> > +	 * Wake up kthreads because there might be some pending messages
> > +	 * added by other CPUs with normal priority since the last flush
> > +	 * in the emergency context.
> > +	 */
> > +	if (!WARN_ON_ONCE(atomic_read(&nbcon_cpu_emergency_cnt) == 0))
> > +		if (atomic_dec_return(&nbcon_cpu_emergency_cnt) == 0)
> > +			nbcon_kthreads_wake();
> 
> Although technically it doesn't hurt to blindly call
> nbcon_kthreads_wake(), you may want to do it more formally. Maybe like
> this:
> 
> 	if (!WARN_ON_ONCE(atomic_read(&nbcon_cpu_emergency_cnt) == 0)) {
> 		if (atomic_dec_return(&nbcon_cpu_emergency_cnt) == 0) {
> 			struct console_flush_type ft;
> 
> 			printk_get_console_flush_type(&ft);
> 			if (ft.nbcon_offload)
> 				nbcon_kthreads_wake();
> 		}
> 	}
> 
> I leave it up to you.

I agree that this is better. I'll use it in v2.

> With the static+initializer change:
> 
> Reviewed-by: John Ogness <john.ogness@linutronix.de>

Thanks a lot for quick review.

I am going to send v2 when the panic state API patchset (in -mm tree)
gets accepted upstream.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context
  2025-09-26 12:49 ` [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context Petr Mladek
  2025-09-26 14:37   ` John Ogness
  2025-09-29  8:40   ` Andrew Murray
@ 2025-09-30 20:15   ` kernel test robot
  2 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-09-30 20:15 UTC (permalink / raw)
  To: Petr Mladek, John Ogness
  Cc: oe-kbuild-all, Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray, Petr Mladek

Hi Petr,

kernel test robot noticed the following build warnings:

[auto build test WARNING on soc/for-next]
[also build test WARNING on linus/master v6.17 next-20250929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Petr-Mladek/printk-nbcon-Block-printk-kthreads-when-any-CPU-is-in-an-emergency-context/20250926-205414
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20250926124912.243464-2-pmladek%40suse.com
patch subject: [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context
config: arc-randconfig-r131-20251001 (https://download.01.org/0day-ci/archive/20251001/202510010320.jV84a9vM-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251001/202510010320.jV84a9vM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510010320.jV84a9vM-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> kernel/printk/nbcon.c:121:10: sparse: sparse: symbol 'nbcon_cpu_emergency_cnt' was not declared. Should it be static?

vim +/nbcon_cpu_emergency_cnt +121 kernel/printk/nbcon.c

     4	
     5	#include <linux/atomic.h>
     6	#include <linux/bug.h>
     7	#include <linux/console.h>
     8	#include <linux/delay.h>
     9	#include <linux/errno.h>
    10	#include <linux/export.h>
    11	#include <linux/init.h>
    12	#include <linux/irqflags.h>
    13	#include <linux/kthread.h>
    14	#include <linux/minmax.h>
    15	#include <linux/percpu.h>
    16	#include <linux/preempt.h>
    17	#include <linux/slab.h>
    18	#include <linux/smp.h>
    19	#include <linux/stddef.h>
    20	#include <linux/string.h>
    21	#include <linux/types.h>
    22	#include "internal.h"
    23	#include "printk_ringbuffer.h"
    24	/*
    25	 * Printk console printing implementation for consoles which does not depend
    26	 * on the legacy style console_lock mechanism.
    27	 *
    28	 * The state of the console is maintained in the "nbcon_state" atomic
    29	 * variable.
    30	 *
    31	 * The console is locked when:
    32	 *
    33	 *   - The 'prio' field contains the priority of the context that owns the
    34	 *     console. Only higher priority contexts are allowed to take over the
    35	 *     lock. A value of 0 (NBCON_PRIO_NONE) means the console is not locked.
    36	 *
    37	 *   - The 'cpu' field denotes on which CPU the console is locked. It is used
    38	 *     to prevent busy waiting on the same CPU. Also it informs the lock owner
    39	 *     that it has lost the lock in a more complex scenario when the lock was
    40	 *     taken over by a higher priority context, released, and taken on another
    41	 *     CPU with the same priority as the interrupted owner.
    42	 *
    43	 * The acquire mechanism uses a few more fields:
    44	 *
    45	 *   - The 'req_prio' field is used by the handover approach to make the
    46	 *     current owner aware that there is a context with a higher priority
    47	 *     waiting for the friendly handover.
    48	 *
    49	 *   - The 'unsafe' field allows to take over the console in a safe way in the
    50	 *     middle of emitting a message. The field is set only when accessing some
    51	 *     shared resources or when the console device is manipulated. It can be
    52	 *     cleared, for example, after emitting one character when the console
    53	 *     device is in a consistent state.
    54	 *
    55	 *   - The 'unsafe_takeover' field is set when a hostile takeover took the
    56	 *     console in an unsafe state. The console will stay in the unsafe state
    57	 *     until re-initialized.
    58	 *
    59	 * The acquire mechanism uses three approaches:
    60	 *
    61	 *   1) Direct acquire when the console is not owned or is owned by a lower
    62	 *      priority context and is in a safe state.
    63	 *
    64	 *   2) Friendly handover mechanism uses a request/grant handshake. It is used
    65	 *      when the current owner has lower priority and the console is in an
    66	 *      unsafe state.
    67	 *
    68	 *      The requesting context:
    69	 *
    70	 *        a) Sets its priority into the 'req_prio' field.
    71	 *
    72	 *        b) Waits (with a timeout) for the owning context to unlock the
    73	 *           console.
    74	 *
    75	 *        c) Takes the lock and clears the 'req_prio' field.
    76	 *
    77	 *      The owning context:
    78	 *
    79	 *        a) Observes the 'req_prio' field set on exit from the unsafe
    80	 *           console state.
    81	 *
    82	 *        b) Gives up console ownership by clearing the 'prio' field.
    83	 *
    84	 *   3) Unsafe hostile takeover allows to take over the lock even when the
    85	 *      console is an unsafe state. It is used only in panic() by the final
    86	 *      attempt to flush consoles in a try and hope mode.
    87	 *
    88	 *      Note that separate record buffers are used in panic(). As a result,
    89	 *      the messages can be read and formatted without any risk even after
    90	 *      using the hostile takeover in unsafe state.
    91	 *
    92	 * The release function simply clears the 'prio' field.
    93	 *
    94	 * All operations on @console::nbcon_state are atomic cmpxchg based to
    95	 * handle concurrency.
    96	 *
    97	 * The acquire/release functions implement only minimal policies:
    98	 *
    99	 *   - Preference for higher priority contexts.
   100	 *   - Protection of the panic CPU.
   101	 *
   102	 * All other policy decisions must be made at the call sites:
   103	 *
   104	 *   - What is marked as an unsafe section.
   105	 *   - Whether to spin-wait if there is already an owner and the console is
   106	 *     in an unsafe state.
   107	 *   - Whether to attempt an unsafe hostile takeover.
   108	 *
   109	 * The design allows to implement the well known:
   110	 *
   111	 *     acquire()
   112	 *     output_one_printk_record()
   113	 *     release()
   114	 *
   115	 * The output of one printk record might be interrupted with a higher priority
   116	 * context. The new owner is supposed to reprint the entire interrupted record
   117	 * from scratch.
   118	 */
   119	
   120	/* Counter of active nbcon emergency contexts. */
 > 121	atomic_t nbcon_cpu_emergency_cnt;
   122	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush
  2025-09-26 12:49 [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush Petr Mladek
                   ` (2 preceding siblings ...)
  2025-09-26 12:49 ` [PATCH 3/3] printk/nbcon: Release nbcon consoles ownership in atomic flush after each emitted record Petr Mladek
@ 2025-10-30 11:32 ` Petr Mladek
  3 siblings, 0 replies; 13+ messages in thread
From: Petr Mladek @ 2025-10-30 11:32 UTC (permalink / raw)
  To: John Ogness
  Cc: Greg Kroah-Hartman, Jiri Slaby, Sergey Senozhatsky,
	Steven Rostedt, Thomas Gleixner, Esben Haabendal, linux-serial,
	linux-kernel, Andy Shevchenko, Arnd Bergmann, Tony Lindgren,
	Niklas Schnelle, Serge Semin, Andrew Murray

On Fri 2025-09-26 14:49:09, Petr Mladek wrote:
> This patchset should solve problem which was being discussed
> at https://lore.kernel.org/all/aNFR45fL2L4PavNc@pathway.suse.cz
> 
> __nbcon_atomic_flush_pending_con() preserves the nbcon console
> ownership all the time when flushing pending messages. It might
> take a long time with slow serial consoles.
> 
> It might trigger a hardlockup report on another CPU which is
> busy waiting for the nbcon console ownership, for example,
> in nbcon_reacquire_nobuf() or __uart_port_nbcon_acquire().
> 
> The problem is solved by the 3rd patch. It releases the console
> context ownership after each record.
> 
> The 3rd patch alone would increase the risk of takeovers and repeated
> lines. It is prevented by the 1st patch which blocks the printk kthread
> when any CPU is in an emergency context.
> 
> The 2nd patch allows to block the printk kthread also in panic.
> It is not important. It is just an obvious update of the check
> for emergency contexts.
> 
> Note: The patchset applies against current Linus' tree (v6.17-rc7).
> 
>       The 2nd patch would need an update after the consolisation of
>       the panic state API gets merged via -mm tree,
>       see https://lore.kernel.org/r/20250825022947.1596226-2-wangjinchao600@gmail.com
> 
> Petr Mladek (3):
>   printk/nbcon: Block printk kthreads when any CPU is in an emergency
>     context
>   printk/nbcon/panic: Allow printk kthread to sleep when the system is
>     in panic
>   printk/nbcon: Release nbcon consoles ownership in atomic flush after
>     each emitted record
> 
>  kernel/printk/internal.h |  1 +
>  kernel/printk/nbcon.c    | 43 +++++++++++++++++++++++++++++++++++-----
>  kernel/printk/printk.c   |  2 +-
>  3 files changed, 40 insertions(+), 6 deletions(-)

JFYI, the patchset has been comitted into printk/linux.git,
branch rework/atomic-flush-hardlockup[1].

It is queued for 6.19.

Note that I did the following modifications:

  + Added changes into the 1st patch proposed by John[2], namely:
     + initialize nbcon_cpu_emergency_cnt and make it static.
     + call nbcon_kthreads_wake() only when printk_get_console_flush_type()
       sets ft.nbcon_offload.

  + Rebased 2nd patch on top of 6.18-rc1 (panic_in_progress() moved to
    linux/panic.h).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git/log/?h=rework/atomic-flush-hardlockup
[2] https://lore.kernel.org/all/841pnti8k2.fsf@jogness.linutronix.de/

Best Regards,
Petr

PS: I thought about sending v2. But v1 already got enough Acks and
    I added the requested changes by cut&paste.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-10-30 11:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-26 12:49 [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush Petr Mladek
2025-09-26 12:49 ` [PATCH 1/3] printk/nbcon: Block printk kthreads when any CPU is in an emergency context Petr Mladek
2025-09-26 14:37   ` John Ogness
2025-09-29 12:02     ` Petr Mladek
2025-09-29  8:40   ` Andrew Murray
2025-09-30 20:15   ` kernel test robot
2025-09-26 12:49 ` [PATCH 2/3] printk/nbcon/panic: Allow printk kthread to sleep when the system is in panic Petr Mladek
2025-09-26 14:38   ` John Ogness
2025-09-29  8:39   ` Andrew Murray
2025-09-26 12:49 ` [PATCH 3/3] printk/nbcon: Release nbcon consoles ownership in atomic flush after each emitted record Petr Mladek
2025-09-26 14:43   ` John Ogness
2025-09-29  8:38   ` Andrew Murray
2025-10-30 11:32 ` [PATCH 0/3] printk/nbcon: Prevent hardlockup reports caused by atomic nbcon flush Petr Mladek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).