* [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq()
[not found] ` <cover.1778592805.git.jnilo@free.fr>
@ 2026-05-12 13:46 ` Jacques Nilo
2026-05-13 11:49 ` Ilpo Järvinen
2026-05-12 13:46 ` [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
2026-05-13 13:30 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
2 siblings, 1 reply; 10+ messages in thread
From: Jacques Nilo @ 2026-05-12 13:46 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold,
Jacques Nilo, stable
serial8250_handle_irq() captures a SysRq character into port->sysrq_ch
inside serial8250_handle_irq_locked() via uart_prepare_sysrq_char()
(reached from serial8250_read_char()). Dispatch of that captured
character to handle_sysrq() is expected to happen at port-unlock time,
through uart_unlock_and_check_sysrq[_irqrestore]().
After commit 8324a54f604d ("serial: 8250: Add
serial8250_handle_irq_locked()") the function was reduced to a wrapper
that takes the port lock via guard(uart_port_lock_irqsave) whose
destructor is plain uart_port_unlock_irqrestore(). The sysrq-aware
unlock helper is no longer called, so port->sysrq_ch is captured but
never dispatched: BREAK + SysRq key is consumed silently.
This was the very condition Johan Hovold's 853a9ae29e978 ("serial:
8250: fix handle_irq locking", 2021) introduced
uart_unlock_and_check_sysrq_irqrestore() to address.
Switch to the new guard(uart_port_lock_sysrq_irqsave), whose destructor
is the sysrq-aware unlock helper, restoring the pre-split behaviour.
Update the Context: comment on serial8250_handle_irq_locked() so future
HW-specific 8250 wrappers know to use the same guard or the explicit
sysrq-aware unlock.
Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
the console UART produces the SysRq help dump in dmesg and the brk
counter in /proc/tty/driver/serial increments correctly.
Fixes: 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
Cc: stable@vger.kernel.org
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
drivers/tty/serial/8250/8250_port.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index e4e6a53eb..64f3487e8 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1786,7 +1786,10 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
}
/*
- * Context: port's lock must be held by the caller.
+ * Context: port's lock must be held by the caller. The caller must
+ * release it via guard(uart_port_lock_sysrq_irqsave) or
+ * uart_unlock_and_check_sysrq_irqrestore(), which captures SysRq
+ * character on unlock.
*/
void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
{
@@ -1839,7 +1842,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
if (iir & UART_IIR_NO_INT)
return 0;
- guard(uart_port_lock_irqsave)(port);
+ guard(uart_port_lock_sysrq_irqsave)(port);
serial8250_handle_irq_locked(port, iir);
return 1;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
[not found] ` <cover.1778592805.git.jnilo@free.fr>
2026-05-12 13:46 ` [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
@ 2026-05-12 13:46 ` Jacques Nilo
2026-05-13 11:50 ` Ilpo Järvinen
2026-05-13 13:30 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
2 siblings, 1 reply; 10+ messages in thread
From: Jacques Nilo @ 2026-05-12 13:46 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Ilpo Järvinen, Johan Hovold,
Jacques Nilo, stable
dw8250_handle_irq() calls serial8250_handle_irq_locked() with the port
lock held via guard(uart_port_lock_irqsave). The guard destructor is
plain uart_port_unlock_irqrestore(), so a SysRq character captured into
port->sysrq_ch by uart_prepare_sysrq_char() is dropped without ever
being dispatched to handle_sysrq().
This is the same regression pattern as in serial8250_handle_irq(),
introduced when 883c5a2bc934 ("serial: 8250_dw: Rework
dw8250_handle_irq() locking and IIR handling") moved the function to
the guard()-based locking scheme without using the sysrq-aware unlock
helper.
Switch to guard(uart_port_lock_sysrq_irqsave) so that captured
sysrq_ch is dispatched on scope exit, matching the fix in
serial8250_handle_irq().
Fixes: 883c5a2bc934 ("serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling")
Cc: stable@vger.kernel.org
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
drivers/tty/serial/8250/8250_dw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 55e40c10f..237543fa7 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -416,7 +416,7 @@ static int dw8250_handle_irq(struct uart_port *p)
unsigned int quirks = d->pdata->quirks;
unsigned int status;
- guard(uart_port_lock_irqsave)(p);
+ guard(uart_port_lock_sysrq_irqsave)(p);
switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
case UART_IIR_NO_INT:
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq()
2026-05-12 13:46 ` [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
@ 2026-05-13 11:49 ` Ilpo Järvinen
0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2026-05-13 11:49 UTC (permalink / raw)
To: Jacques Nilo
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML, Johan Hovold,
stable
[-- Attachment #1: Type: text/plain, Size: 2858 bytes --]
On Tue, 12 May 2026, Jacques Nilo wrote:
> serial8250_handle_irq() captures a SysRq character into port->sysrq_ch
> inside serial8250_handle_irq_locked() via uart_prepare_sysrq_char()
> (reached from serial8250_read_char()). Dispatch of that captured
> character to handle_sysrq() is expected to happen at port-unlock time,
> through uart_unlock_and_check_sysrq[_irqrestore]().
>
> After commit 8324a54f604d ("serial: 8250: Add
> serial8250_handle_irq_locked()") the function was reduced to a wrapper
> that takes the port lock via guard(uart_port_lock_irqsave) whose
> destructor is plain uart_port_unlock_irqrestore(). The sysrq-aware
> unlock helper is no longer called, so port->sysrq_ch is captured but
> never dispatched: BREAK + SysRq key is consumed silently.
>
> This was the very condition Johan Hovold's 853a9ae29e978 ("serial:
> 8250: fix handle_irq locking", 2021) introduced
> uart_unlock_and_check_sysrq_irqrestore() to address.
>
> Switch to the new guard(uart_port_lock_sysrq_irqsave), whose destructor
> is the sysrq-aware unlock helper, restoring the pre-split behaviour.
> Update the Context: comment on serial8250_handle_irq_locked() so future
> HW-specific 8250 wrappers know to use the same guard or the explicit
> sysrq-aware unlock.
>
> Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
> the console UART produces the SysRq help dump in dmesg and the brk
> counter in /proc/tty/driver/serial increments correctly.
>
> Fixes: 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jacques Nilo <jnilo@free.fr>
> ---
> drivers/tty/serial/8250/8250_port.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index e4e6a53eb..64f3487e8 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1786,7 +1786,10 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
> }
>
> /*
> - * Context: port's lock must be held by the caller.
> + * Context: port's lock must be held by the caller. The caller must
> + * release it via guard(uart_port_lock_sysrq_irqsave) or
> + * uart_unlock_and_check_sysrq_irqrestore(), which captures SysRq
> + * character on unlock.
> */
> void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
> {
> @@ -1839,7 +1842,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
> if (iir & UART_IIR_NO_INT)
> return 0;
>
> - guard(uart_port_lock_irqsave)(port);
> + guard(uart_port_lock_sysrq_irqsave)(port);
> serial8250_handle_irq_locked(port, iir);
>
> return 1;
>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
2026-05-12 13:46 ` [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
@ 2026-05-13 11:50 ` Ilpo Järvinen
0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2026-05-13 11:50 UTC (permalink / raw)
To: Jacques Nilo
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, LKML, Johan Hovold,
stable
[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]
On Tue, 12 May 2026, Jacques Nilo wrote:
> dw8250_handle_irq() calls serial8250_handle_irq_locked() with the port
> lock held via guard(uart_port_lock_irqsave). The guard destructor is
> plain uart_port_unlock_irqrestore(), so a SysRq character captured into
> port->sysrq_ch by uart_prepare_sysrq_char() is dropped without ever
> being dispatched to handle_sysrq().
>
> This is the same regression pattern as in serial8250_handle_irq(),
> introduced when 883c5a2bc934 ("serial: 8250_dw: Rework
> dw8250_handle_irq() locking and IIR handling") moved the function to
> the guard()-based locking scheme without using the sysrq-aware unlock
> helper.
>
> Switch to guard(uart_port_lock_sysrq_irqsave) so that captured
> sysrq_ch is dispatched on scope exit, matching the fix in
> serial8250_handle_irq().
>
> Fixes: 883c5a2bc934 ("serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jacques Nilo <jnilo@free.fr>
> ---
> drivers/tty/serial/8250/8250_dw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 55e40c10f..237543fa7 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -416,7 +416,7 @@ static int dw8250_handle_irq(struct uart_port *p)
> unsigned int quirks = d->pdata->quirks;
> unsigned int status;
>
> - guard(uart_port_lock_irqsave)(p);
> + guard(uart_port_lock_sysrq_irqsave)(p);
>
> switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
> case UART_IIR_NO_INT:
>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers
[not found] ` <cover.1778592805.git.jnilo@free.fr>
2026-05-12 13:46 ` [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
2026-05-12 13:46 ` [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
@ 2026-05-13 13:30 ` Jacques Nilo
2026-05-13 13:30 ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
` (3 more replies)
2 siblings, 4 replies; 10+ messages in thread
From: Jacques Nilo @ 2026-05-13 13:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, linux-kernel,
stable, Jacques Nilo
This series fixes a silent regression where a SysRq character entered as
BREAK + key on the serial console is consumed by the kernel but never
dispatched to handle_sysrq(). Same description as v1 [1].
v1 -> v2 (per Ilpo's review [2]):
- Renamed the new lock guard from uart_port_lock_sysrq_irqsave to
uart_port_lock_check_sysrq_irqsave, preserving the "check" semantics
of the destructor's underlying helper
uart_unlock_and_check_sysrq_irqrestore(). Mechanical rename across
patches 2/3 and 3/3; Ilpo's Reviewed-by trailers from v1 carried
forward.
- Patch 1/3 commit message reflowed: the "guard(...)" form is spelled
out, the "lock side is identical" sentence moved up next to the
variant introduction, the now-redundant naming-rationale sentence
removed, and "opt in by using" tightened to "must use".
- Added Cc: stable@vger.kernel.org to patch 1/3 (prerequisite for the
stable backport of 2/3 and 3/3); no Fixes: tag, since 1/3 adds new
API rather than fixing existing code.
- Collapsed the DEFINE_LOCK_GUARD_1 destructor expression to a single
line, which fits within the expected indentation.
No re-test of the BREAK + 'h' path was performed for v2 since the
diff against v1 is purely a textual rename plus the commit-message
reflow above; the v1 RTL8196E validation (BREAK + 'h' on the console
UART producing the SysRq help dump, brk counter incrementing in
/proc/tty/driver/serial) continues to apply unchanged. Built and
booted on tty-next (base 16e95bfb79b5).
[1] https://lore.kernel.org/linux-serial/cover.1778592805.git.jnilo@free.fr/
[2] https://lore.kernel.org/linux-serial/3439217b-90b5-5d21-e777-d238b3ffc1a0@linux.intel.com/
Jacques Nilo (3):
serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave)
serial: 8250: dispatch SysRq character in serial8250_handle_irq()
serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
drivers/tty/serial/8250/8250_dw.c | 2 +-
drivers/tty/serial/8250/8250_port.c | 7 +++++--
include/linux/serial_core.h | 12 ++++++++++++
3 files changed, 18 insertions(+), 3 deletions(-)
base-commit: 16e95bfb79b5d9d01dc7651d98caf3c2ace331cd
--
2.43.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave)
2026-05-13 13:30 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
@ 2026-05-13 13:30 ` Jacques Nilo
2026-05-13 13:35 ` Ilpo Järvinen
2026-05-13 13:30 ` [PATCH v2 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Jacques Nilo @ 2026-05-13 13:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, linux-kernel,
stable, Jacques Nilo
uart_handle_break() and uart_prepare_sysrq_char() (in
include/linux/serial_core.h) capture a SysRq character into
port->sysrq_ch while the port lock is held and rely on the unlock
helper -- uart_unlock_and_check_sysrq_irqrestore() -- to dispatch the
captured character to handle_sysrq() on scope exit.
The existing guard(uart_port_lock_irqsave) cannot be used by IRQ
handlers that process RX, because its destructor calls plain
uart_port_unlock_irqrestore() and silently drops port->sysrq_ch.
Add a dedicated guard(uart_port_lock_check_sysrq_irqsave) variant
whose destructor is the sysrq-aware unlock helper. The lock side is
identical to uart_port_lock_irqsave -- only the unlock-time behaviour
differs. Callers that may capture SysRq characters must use
guard(uart_port_lock_check_sysrq_irqsave); the existing
guard(uart_port_lock_irqsave) keeps its current plain-unlock semantics
for the many callers that do not process RX.
The new macro is placed after the CONFIG_MAGIC_SYSRQ_SERIAL block so
both definitions of uart_unlock_and_check_sysrq_irqrestore() (sysrq
enabled and disabled) are visible at expansion time. When
CONFIG_MAGIC_SYSRQ_SERIAL=n the destructor degenerates to plain
uart_port_unlock_irqrestore(), so there is no overhead.
No functional change on its own; users are converted in the following
patches.
Cc: stable@vger.kernel.org
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
include/linux/serial_core.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 4f7bbdd90..d1404c97d 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -1286,6 +1286,18 @@ static inline void uart_unlock_and_check_sysrq_irqrestore(struct uart_port *port
}
#endif /* CONFIG_MAGIC_SYSRQ_SERIAL */
+/*
+ * Variant of guard(uart_port_lock_irqsave) for IRQ handlers that may capture
+ * a SysRq character via uart_prepare_sysrq_char(). The destructor uses the
+ * sysrq-aware unlock helper so that a captured port->sysrq_ch is dispatched
+ * to handle_sysrq() on scope exit. The plain guard variant silently drops
+ * sysrq_ch and must not be used by callers that process RX.
+ */
+DEFINE_LOCK_GUARD_1(uart_port_lock_check_sysrq_irqsave, struct uart_port,
+ uart_port_lock_irqsave(_T->lock, &_T->flags),
+ uart_unlock_and_check_sysrq_irqrestore(_T->lock, _T->flags),
+ unsigned long flags);
+
/*
* We do the SysRQ and SAK checking like this...
*/
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq()
2026-05-13 13:30 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
2026-05-13 13:30 ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
@ 2026-05-13 13:30 ` Jacques Nilo
2026-05-13 13:30 ` [PATCH v2 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
2026-05-13 17:51 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Andy Shevchenko
3 siblings, 0 replies; 10+ messages in thread
From: Jacques Nilo @ 2026-05-13 13:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, linux-kernel,
stable, Jacques Nilo
serial8250_handle_irq() captures a SysRq character into port->sysrq_ch
inside serial8250_handle_irq_locked() via uart_prepare_sysrq_char()
(reached from serial8250_read_char()). Dispatch of that captured
character to handle_sysrq() is expected to happen at port-unlock time,
through uart_unlock_and_check_sysrq[_irqrestore]().
After commit 8324a54f604d ("serial: 8250: Add
serial8250_handle_irq_locked()") the function was reduced to a wrapper
that takes the port lock via guard(uart_port_lock_irqsave) whose
destructor is plain uart_port_unlock_irqrestore(). The sysrq-aware
unlock helper is no longer called, so port->sysrq_ch is captured but
never dispatched: BREAK + SysRq key is consumed silently.
This was the very condition Johan Hovold's 853a9ae29e978 ("serial:
8250: fix handle_irq locking", 2021) introduced
uart_unlock_and_check_sysrq_irqrestore() to address.
Switch to the new guard(uart_port_lock_check_sysrq_irqsave), whose
destructor is the sysrq-aware unlock helper, restoring the pre-split
behaviour. Update the Context: comment on serial8250_handle_irq_locked()
so future HW-specific 8250 wrappers know to use the same guard or the
explicit sysrq-aware unlock.
Verified on RTL8196E with CONFIG_MAGIC_SYSRQ_SERIAL=y: BREAK + 'h' on
the console UART produces the SysRq help dump in dmesg and the brk
counter in /proc/tty/driver/serial increments correctly.
Fixes: 8324a54f604d ("serial: 8250: Add serial8250_handle_irq_locked()")
Cc: stable@vger.kernel.org
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
drivers/tty/serial/8250/8250_port.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index e4e6a53eb..59203bbfb 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1786,7 +1786,10 @@ static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
}
/*
- * Context: port's lock must be held by the caller.
+ * Context: port's lock must be held by the caller. The caller must
+ * release it via guard(uart_port_lock_check_sysrq_irqsave) or
+ * uart_unlock_and_check_sysrq_irqrestore(), which captures SysRq
+ * character on unlock.
*/
void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
{
@@ -1839,7 +1842,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
if (iir & UART_IIR_NO_INT)
return 0;
- guard(uart_port_lock_irqsave)(port);
+ guard(uart_port_lock_check_sysrq_irqsave)(port);
serial8250_handle_irq_locked(port, iir);
return 1;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq()
2026-05-13 13:30 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
2026-05-13 13:30 ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
2026-05-13 13:30 ` [PATCH v2 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
@ 2026-05-13 13:30 ` Jacques Nilo
2026-05-13 17:51 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Andy Shevchenko
3 siblings, 0 replies; 10+ messages in thread
From: Jacques Nilo @ 2026-05-13 13:30 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Ilpo Järvinen, Andy Shevchenko, linux-serial, linux-kernel,
stable, Jacques Nilo
dw8250_handle_irq() calls serial8250_handle_irq_locked() with the port
lock held via guard(uart_port_lock_irqsave). The guard destructor is
plain uart_port_unlock_irqrestore(), so a SysRq character captured into
port->sysrq_ch by uart_prepare_sysrq_char() is dropped without ever
being dispatched to handle_sysrq().
This is the same regression pattern as in serial8250_handle_irq(),
introduced when 883c5a2bc934 ("serial: 8250_dw: Rework
dw8250_handle_irq() locking and IIR handling") moved the function to
the guard()-based locking scheme without using the sysrq-aware unlock
helper.
Switch to guard(uart_port_lock_check_sysrq_irqsave) so that captured
sysrq_ch is dispatched on scope exit, matching the fix in
serial8250_handle_irq().
Fixes: 883c5a2bc934 ("serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling")
Cc: stable@vger.kernel.org
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jacques Nilo <jnilo@free.fr>
---
drivers/tty/serial/8250/8250_dw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 55e40c10f..9d552b224 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -416,7 +416,7 @@ static int dw8250_handle_irq(struct uart_port *p)
unsigned int quirks = d->pdata->quirks;
unsigned int status;
- guard(uart_port_lock_irqsave)(p);
+ guard(uart_port_lock_check_sysrq_irqsave)(p);
switch (FIELD_GET(DW_UART_IIR_IID, iir)) {
case UART_IIR_NO_INT:
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave)
2026-05-13 13:30 ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
@ 2026-05-13 13:35 ` Ilpo Järvinen
0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2026-05-13 13:35 UTC (permalink / raw)
To: Jacques Nilo
Cc: Greg Kroah-Hartman, Jiri Slaby, Andy Shevchenko, linux-serial,
LKML, stable
[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]
On Wed, 13 May 2026, Jacques Nilo wrote:
> uart_handle_break() and uart_prepare_sysrq_char() (in
> include/linux/serial_core.h) capture a SysRq character into
> port->sysrq_ch while the port lock is held and rely on the unlock
> helper -- uart_unlock_and_check_sysrq_irqrestore() -- to dispatch the
> captured character to handle_sysrq() on scope exit.
>
> The existing guard(uart_port_lock_irqsave) cannot be used by IRQ
> handlers that process RX, because its destructor calls plain
> uart_port_unlock_irqrestore() and silently drops port->sysrq_ch.
>
> Add a dedicated guard(uart_port_lock_check_sysrq_irqsave) variant
> whose destructor is the sysrq-aware unlock helper. The lock side is
> identical to uart_port_lock_irqsave -- only the unlock-time behaviour
> differs. Callers that may capture SysRq characters must use
> guard(uart_port_lock_check_sysrq_irqsave); the existing
> guard(uart_port_lock_irqsave) keeps its current plain-unlock semantics
> for the many callers that do not process RX.
>
> The new macro is placed after the CONFIG_MAGIC_SYSRQ_SERIAL block so
> both definitions of uart_unlock_and_check_sysrq_irqrestore() (sysrq
> enabled and disabled) are visible at expansion time. When
> CONFIG_MAGIC_SYSRQ_SERIAL=n the destructor degenerates to plain
> uart_port_unlock_irqrestore(), so there is no overhead.
>
> No functional change on its own; users are converted in the following
> patches.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Jacques Nilo <jnilo@free.fr>
> ---
> include/linux/serial_core.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 4f7bbdd90..d1404c97d 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -1286,6 +1286,18 @@ static inline void uart_unlock_and_check_sysrq_irqrestore(struct uart_port *port
> }
> #endif /* CONFIG_MAGIC_SYSRQ_SERIAL */
>
> +/*
> + * Variant of guard(uart_port_lock_irqsave) for IRQ handlers that may capture
> + * a SysRq character via uart_prepare_sysrq_char(). The destructor uses the
> + * sysrq-aware unlock helper so that a captured port->sysrq_ch is dispatched
> + * to handle_sysrq() on scope exit. The plain guard variant silently drops
> + * sysrq_ch and must not be used by callers that process RX.
> + */
> +DEFINE_LOCK_GUARD_1(uart_port_lock_check_sysrq_irqsave, struct uart_port,
> + uart_port_lock_irqsave(_T->lock, &_T->flags),
> + uart_unlock_and_check_sysrq_irqrestore(_T->lock, _T->flags),
> + unsigned long flags);
> +
> /*
> * We do the SysRQ and SAK checking like this...
> */
>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers
2026-05-13 13:30 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
` (2 preceding siblings ...)
2026-05-13 13:30 ` [PATCH v2 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
@ 2026-05-13 17:51 ` Andy Shevchenko
3 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2026-05-13 17:51 UTC (permalink / raw)
To: Jacques Nilo
Cc: Greg Kroah-Hartman, Jiri Slaby, Ilpo Järvinen, linux-serial,
linux-kernel, stable
On Wed, May 13, 2026 at 03:30:22PM +0200, Jacques Nilo wrote:
> This series fixes a silent regression where a SysRq character entered as
> BREAK + key on the serial console is consumed by the kernel but never
> dispatched to handle_sysrq(). Same description as v1 [1].
I have read the v1 discussion and v2 makes sense to me and looks good
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-05-13 17:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5efe9e03-4d86-43a0-9ec2-e610ff31095d@free.fr>
[not found] ` <cover.1778592805.git.jnilo@free.fr>
2026-05-12 13:46 ` [PATCH 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
2026-05-13 11:49 ` Ilpo Järvinen
2026-05-12 13:46 ` [PATCH 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
2026-05-13 11:50 ` Ilpo Järvinen
2026-05-13 13:30 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Jacques Nilo
2026-05-13 13:30 ` [PATCH v2 1/3] serial: core: introduce guard(uart_port_lock_check_sysrq_irqsave) Jacques Nilo
2026-05-13 13:35 ` Ilpo Järvinen
2026-05-13 13:30 ` [PATCH v2 2/3] serial: 8250: dispatch SysRq character in serial8250_handle_irq() Jacques Nilo
2026-05-13 13:30 ` [PATCH v2 3/3] serial: 8250_dw: dispatch SysRq character in dw8250_handle_irq() Jacques Nilo
2026-05-13 17:51 ` [PATCH v2 0/3] serial: 8250: fix BREAK+SysRq dispatch on guard()-locked IRQ handlers Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox