Re: [PATCH 1/3] serial: core: introduce guard(uart_port_lock_sysrq_irqsave)

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Tue, 12 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 variant whose destructor is the sysrq-aware

guard(...) (put the full form here already).

> unlock helper. Callers that may capture SysRq characters opt in by
> using guard(uart_port_lock_sysrq_irqsave); the existing

opt in by using -> must use

> uart_port_lock_irqsave guard keeps its current plain-unlock semantics

guard(uart_port_lock_irqsave)

> for the many callers that do not process RX.
> 
> The lock side is identical to uart_port_lock_irqsave -- only the
> unlock-time behaviour differs.

Move this to the previous paragraph after the first sentence.

> Naming mirrors
> uart_unlock_and_check_sysrq_irqrestore() (sysrq before irqsave/irqrestore).

I'd remove the sentence about the name, it's kind of obvious.

> 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.
> 

You should add Cc: stable here too as this is prerequisite for the other 
two patches (but no Fixes tag).

> Signed-off-by: Jacques Nilo <jnilo@free.fr>
> ---
>  include/linux/serial_core.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 4f7bbdd90..4fa079dc9 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -1286,6 +1286,19 @@ 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_sysrq_irqsave, struct uart_port,

I suppose the "check" in the name is kind of important detail so maybe it 
shouldn't be dropped from the guard name.

> +                    uart_port_lock_irqsave(_T->lock, &_T->flags),
> +                    uart_unlock_and_check_sysrq_irqrestore(_T->lock,
> +                                                           _T->flags),

This fits to one line.

> +                    unsigned long flags);
> +
>  /*
>   * We do the SysRQ and SAK checking like this...
>   */
> 

-- 
 i.