* [PATCH 00/16] tty: use lock, rpm, and free guards
@ 2025-08-14 7:24 Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 01/16] console: introduce console_lock guard()s Jiri Slaby (SUSE)
` (15 more replies)
0 siblings, 16 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
The first 4 patches introduce guards for:
* console_lock/console_unlock
* tty_port_tty_get/tty_kref_put
* uart_port_lock/uart_port_unlock in all its variants (_irq, _irqsave,
_try)
* serial8250_rpm_get/serial8250_rpm_put
"tty/vt: use guard()s" also introduces a local free_page_ptr guard for
__get_free_page/free_page (with proper casts). This could be made
public in include/.
The rest of patches make uses all those guards across the tty code.
Jiri Slaby (SUSE) (16):
console: introduce console_lock guard()s
tty: introduce tty_port_tty guard()
serial: introduce uart_port_lock() guard()s
serial: 8250: introduce RPM guard()s
tty: tty_port: use guard()s
mxser: use tty_port_tty guard() in mxser_port_isr()
mxser: use guard()s
serial: serial_core: use guard()s
serial: 8250: use guard()s
serial: 8250_core: use guard() in serial_unlink_irq_chain()
serial: 8250_omap: extract omap_8250_set_termios_atomic()
serial: 8250_omap: use guard()s
serial: 8250_rsa: use guard()s
tty/vt: use guard()s in con_font_set/get() and con_{set,get}_unimap()
tty/vt: use guard()s
s390/char/con3270: use tty_port_tty guard()
drivers/s390/char/con3270.c | 18 +-
drivers/tty/mxser.c | 259 ++++++++++--------------
drivers/tty/serial/8250/8250.h | 5 +
drivers/tty/serial/8250/8250_core.c | 91 ++++-----
drivers/tty/serial/8250/8250_omap.c | 145 ++++++--------
drivers/tty/serial/8250/8250_port.c | 298 ++++++++++++----------------
drivers/tty/serial/8250/8250_rsa.c | 7 +-
drivers/tty/serial/serial_core.c | 143 ++++++-------
drivers/tty/tty_port.c | 168 +++++++---------
drivers/tty/vt/consolemap.c | 116 +++++------
drivers/tty/vt/selection.c | 20 +-
drivers/tty/vt/vc_screen.c | 74 +++----
drivers/tty/vt/vt.c | 187 ++++++++---------
drivers/tty/vt/vt_ioctl.c | 190 ++++++++----------
include/linux/console.h | 2 +
include/linux/serial_core.h | 13 ++
include/linux/tty_port.h | 14 ++
17 files changed, 747 insertions(+), 1003 deletions(-)
--
2.50.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 01/16] console: introduce console_lock guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 02/16] tty: introduce tty_port_tty guard() Jiri Slaby (SUSE)
` (14 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having this, guards like these work:
guard(console_lock)();
or
scoped_guard(console_lock) {
...
}
See e.g. "vc_screen: use guard()s" later in this series.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
include/linux/console.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/console.h b/include/linux/console.h
index 8f10d0a85bb4..031a58dc2b91 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -666,6 +666,8 @@ void vcs_remove_sysfs(int index);
*/
extern atomic_t ignore_console_lock_warning;
+DEFINE_LOCK_GUARD_0(console_lock, console_lock(), console_unlock());
+
extern void console_init(void);
/* For deferred console takeover */
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 02/16] tty: introduce tty_port_tty guard()
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 01/16] console: introduce console_lock guard()s Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 03/16] serial: introduce uart_port_lock() guard()s Jiri Slaby (SUSE)
` (13 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having this, guards like these work:
scoped_guard(tty_port_tty, port)
tty_wakeup(scoped_tty());
See e.g. "tty_port: use scoped_guard()" later in this series.
The definitions depend on CONFIG_TTY. It's due to tty_kref_put().
On !CONFIG_TTY, it is an inline and its declaration would conflict. The
guards are not needed in that case, of course.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
include/linux/tty_port.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/tty_port.h b/include/linux/tty_port.h
index 332ddb93603e..660c254f1efe 100644
--- a/include/linux/tty_port.h
+++ b/include/linux/tty_port.h
@@ -270,4 +270,18 @@ static inline void tty_port_tty_vhangup(struct tty_port *port)
__tty_port_tty_hangup(port, false, false);
}
+#ifdef CONFIG_TTY
+void tty_kref_put(struct tty_struct *tty);
+__DEFINE_CLASS_IS_CONDITIONAL(tty_port_tty, true);
+__DEFINE_UNLOCK_GUARD(tty_port_tty, struct tty_struct, tty_kref_put(_T->lock));
+static inline class_tty_port_tty_t class_tty_port_tty_constructor(struct tty_port *tport)
+{
+ class_tty_port_tty_t _t = {
+ .lock = tty_port_tty_get(tport),
+ };
+ return _t;
+}
+#define scoped_tty() ((struct tty_struct *)(__guard_ptr(tty_port_tty)(&scope)))
+#endif
+
#endif
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/16] serial: introduce uart_port_lock() guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 01/16] console: introduce console_lock guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 02/16] tty: introduce tty_port_tty guard() Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 04/16] serial: 8250: introduce RPM guard()s Jiri Slaby (SUSE)
` (12 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having this, guards like these work:
guard(uart_port_lock_irq)(&up->port);
or
scoped_guard(uart_port_lock_irqsave, port) {
...
}
See e.g. "serial: 8250: use guard()s" later in this series.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
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 84b4648ead7e..666430b47899 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -788,6 +788,19 @@ static inline void uart_port_unlock_irqrestore(struct uart_port *up, unsigned lo
spin_unlock_irqrestore(&up->lock, flags);
}
+DEFINE_GUARD(uart_port_lock, struct uart_port *, uart_port_lock(_T), uart_port_unlock(_T));
+DEFINE_GUARD_COND(uart_port_lock, _try, uart_port_trylock(_T));
+
+DEFINE_GUARD(uart_port_lock_irq, struct uart_port *, uart_port_lock_irq(_T),
+ uart_port_unlock_irq(_T));
+
+DEFINE_LOCK_GUARD_1(uart_port_lock_irqsave, struct uart_port,
+ uart_port_lock_irqsave(_T->lock, &_T->flags),
+ uart_port_unlock_irqrestore(_T->lock, _T->flags),
+ unsigned long flags);
+DEFINE_LOCK_GUARD_1_COND(uart_port_lock_irqsave, _try,
+ uart_port_trylock_irqsave(_T->lock, &_T->flags));
+
static inline int serial_port_in(struct uart_port *up, int offset)
{
return up->serial_in(up, offset);
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/16] serial: 8250: introduce RPM guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (2 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 03/16] serial: introduce uart_port_lock() guard()s Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 05/16] tty: tty_port: use guard()s Jiri Slaby (SUSE)
` (11 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having this, guards like these work:
guard(serial8250_rpm)(up);
or
scoped_guard(serial8250_rpm, up) {
...
}
See e.g. "serial: 8250: use guard()s" later in this series.
And make them available to anyone (EXPORT + put in 8250.h) as drivers
open code this anyway.
The _tx ones are not defined as they would have no user.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/serial/8250/8250.h | 5 +++++
drivers/tty/serial/8250/8250_port.c | 6 ++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index cfe6ba286b45..58e64c4e1e3a 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -186,6 +186,11 @@ static unsigned int __maybe_unused serial_icr_read(struct uart_8250_port *up,
void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p);
+void serial8250_rpm_get(struct uart_8250_port *p);
+void serial8250_rpm_put(struct uart_8250_port *p);
+DEFINE_GUARD(serial8250_rpm, struct uart_8250_port *,
+ serial8250_rpm_get(_T), serial8250_rpm_put(_T));
+
static inline u32 serial_dl_read(struct uart_8250_port *up)
{
return up->dl_read(up);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 2da9db960d09..5afae4025696 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -508,20 +508,22 @@ void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p)
}
EXPORT_SYMBOL_GPL(serial8250_clear_and_reinit_fifos);
-static void serial8250_rpm_get(struct uart_8250_port *p)
+void serial8250_rpm_get(struct uart_8250_port *p)
{
if (!(p->capabilities & UART_CAP_RPM))
return;
pm_runtime_get_sync(p->port.dev);
}
+EXPORT_SYMBOL_GPL(serial8250_rpm_get);
-static void serial8250_rpm_put(struct uart_8250_port *p)
+void serial8250_rpm_put(struct uart_8250_port *p)
{
if (!(p->capabilities & UART_CAP_RPM))
return;
pm_runtime_mark_last_busy(p->port.dev);
pm_runtime_put_autosuspend(p->port.dev);
}
+EXPORT_SYMBOL_GPL(serial8250_rpm_put);
/**
* serial8250_em485_init() - put uart_8250_port into rs485 emulating
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 05/16] tty: tty_port: use guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (3 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 04/16] serial: 8250: introduce RPM guard()s Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 06/16] mxser: use tty_port_tty guard() in mxser_port_isr() Jiri Slaby (SUSE)
` (10 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having all the new guards, use them in the tty_port code. This improves
readability, makes error handling easier, and marks locked portions of
code explicit.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/tty_port.c | 168 ++++++++++++++++++-----------------------
1 file changed, 74 insertions(+), 94 deletions(-)
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 5b4d5fb99a59..fe67c5cb0a3f 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -63,12 +63,8 @@ static void tty_port_default_lookahead_buf(struct tty_port *port, const u8 *p,
static void tty_port_default_wakeup(struct tty_port *port)
{
- struct tty_struct *tty = tty_port_tty_get(port);
-
- if (tty) {
- tty_wakeup(tty);
- tty_kref_put(tty);
- }
+ scoped_guard(tty_port_tty, port)
+ tty_wakeup(scoped_tty());
}
const struct tty_port_client_operations tty_port_default_client_ops = {
@@ -225,26 +221,27 @@ EXPORT_SYMBOL_GPL(tty_port_unregister_device);
int tty_port_alloc_xmit_buf(struct tty_port *port)
{
/* We may sleep in get_zeroed_page() */
- mutex_lock(&port->buf_mutex);
- if (port->xmit_buf == NULL) {
- port->xmit_buf = (u8 *)get_zeroed_page(GFP_KERNEL);
- if (port->xmit_buf)
- kfifo_init(&port->xmit_fifo, port->xmit_buf, PAGE_SIZE);
- }
- mutex_unlock(&port->buf_mutex);
+ guard(mutex)(&port->buf_mutex);
+
+ if (port->xmit_buf)
+ return 0;
+
+ port->xmit_buf = (u8 *)get_zeroed_page(GFP_KERNEL);
if (port->xmit_buf == NULL)
return -ENOMEM;
+
+ kfifo_init(&port->xmit_fifo, port->xmit_buf, PAGE_SIZE);
+
return 0;
}
EXPORT_SYMBOL(tty_port_alloc_xmit_buf);
void tty_port_free_xmit_buf(struct tty_port *port)
{
- mutex_lock(&port->buf_mutex);
+ guard(mutex)(&port->buf_mutex);
free_page((unsigned long)port->xmit_buf);
port->xmit_buf = NULL;
INIT_KFIFO(port->xmit_fifo);
- mutex_unlock(&port->buf_mutex);
}
EXPORT_SYMBOL(tty_port_free_xmit_buf);
@@ -301,13 +298,8 @@ EXPORT_SYMBOL(tty_port_put);
*/
struct tty_struct *tty_port_tty_get(struct tty_port *port)
{
- unsigned long flags;
- struct tty_struct *tty;
-
- spin_lock_irqsave(&port->lock, flags);
- tty = tty_kref_get(port->tty);
- spin_unlock_irqrestore(&port->lock, flags);
- return tty;
+ guard(spinlock_irqsave)(&port->lock);
+ return tty_kref_get(port->tty);
}
EXPORT_SYMBOL(tty_port_tty_get);
@@ -321,12 +313,9 @@ EXPORT_SYMBOL(tty_port_tty_get);
*/
void tty_port_tty_set(struct tty_port *port, struct tty_struct *tty)
{
- unsigned long flags;
-
- spin_lock_irqsave(&port->lock, flags);
+ guard(spinlock_irqsave)(&port->lock);
tty_kref_put(port->tty);
port->tty = tty_kref_get(tty);
- spin_unlock_irqrestore(&port->lock, flags);
}
EXPORT_SYMBOL(tty_port_tty_set);
@@ -342,24 +331,24 @@ EXPORT_SYMBOL(tty_port_tty_set);
*/
static void tty_port_shutdown(struct tty_port *port, struct tty_struct *tty)
{
- mutex_lock(&port->mutex);
+ guard(mutex)(&port->mutex);
+
if (port->console)
- goto out;
+ return;
- if (tty_port_initialized(port)) {
- tty_port_set_initialized(port, false);
- /*
- * Drop DTR/RTS if HUPCL is set. This causes any attached
- * modem to hang up the line.
- */
- if (tty && C_HUPCL(tty))
- tty_port_lower_dtr_rts(port);
+ if (!tty_port_initialized(port))
+ return;
- if (port->ops->shutdown)
- port->ops->shutdown(port);
- }
-out:
- mutex_unlock(&port->mutex);
+ tty_port_set_initialized(port, false);
+ /*
+ * Drop DTR/RTS if HUPCL is set. This causes any attached
+ * modem to hang up the line.
+ */
+ if (tty && C_HUPCL(tty))
+ tty_port_lower_dtr_rts(port);
+
+ if (port->ops->shutdown)
+ port->ops->shutdown(port);
}
/**
@@ -374,15 +363,15 @@ static void tty_port_shutdown(struct tty_port *port, struct tty_struct *tty)
void tty_port_hangup(struct tty_port *port)
{
struct tty_struct *tty;
- unsigned long flags;
- spin_lock_irqsave(&port->lock, flags);
- port->count = 0;
- tty = port->tty;
- if (tty)
- set_bit(TTY_IO_ERROR, &tty->flags);
- port->tty = NULL;
- spin_unlock_irqrestore(&port->lock, flags);
+ scoped_guard(spinlock_irqsave, &port->lock) {
+ port->count = 0;
+ tty = port->tty;
+ if (tty)
+ set_bit(TTY_IO_ERROR, &tty->flags);
+ port->tty = NULL;
+ }
+
tty_port_set_active(port, false);
tty_port_shutdown(port, tty);
tty_kref_put(tty);
@@ -393,15 +382,16 @@ EXPORT_SYMBOL(tty_port_hangup);
void __tty_port_tty_hangup(struct tty_port *port, bool check_clocal, bool async)
{
- struct tty_struct *tty = tty_port_tty_get(port);
+ scoped_guard(tty_port_tty, port) {
+ struct tty_struct *tty = scoped_tty();
- if (tty && (!check_clocal || !C_CLOCAL(tty))) {
- if (async)
- tty_hangup(tty);
- else
- tty_vhangup(tty);
+ if (!check_clocal || !C_CLOCAL(tty)) {
+ if (async)
+ tty_hangup(tty);
+ else
+ tty_vhangup(tty);
+ }
}
- tty_kref_put(tty);
}
EXPORT_SYMBOL_GPL(__tty_port_tty_hangup);
@@ -490,7 +480,6 @@ int tty_port_block_til_ready(struct tty_port *port,
struct tty_struct *tty, struct file *filp)
{
int do_clocal = 0, retval;
- unsigned long flags;
DEFINE_WAIT(wait);
/* if non-blocking mode is set we can pass directly to open unless
@@ -519,10 +508,10 @@ int tty_port_block_til_ready(struct tty_port *port,
retval = 0;
/* The port lock protects the port counts */
- spin_lock_irqsave(&port->lock, flags);
- port->count--;
- port->blocked_open++;
- spin_unlock_irqrestore(&port->lock, flags);
+ scoped_guard(spinlock_irqsave, &port->lock) {
+ port->count--;
+ port->blocked_open++;
+ }
while (1) {
/* Indicate we are open */
@@ -561,11 +550,11 @@ int tty_port_block_til_ready(struct tty_port *port,
/* Update counts. A parallel hangup will have set count to zero and
* we must not mess that up further.
*/
- spin_lock_irqsave(&port->lock, flags);
- if (!tty_hung_up_p(filp))
- port->count++;
- port->blocked_open--;
- spin_unlock_irqrestore(&port->lock, flags);
+ scoped_guard(spinlock_irqsave, &port->lock) {
+ if (!tty_hung_up_p(filp))
+ port->count++;
+ port->blocked_open--;
+ }
if (retval == 0)
tty_port_set_active(port, true);
return retval;
@@ -604,28 +593,24 @@ static void tty_port_drain_delay(struct tty_port *port, struct tty_struct *tty)
int tty_port_close_start(struct tty_port *port,
struct tty_struct *tty, struct file *filp)
{
- unsigned long flags;
-
if (tty_hung_up_p(filp))
return 0;
- spin_lock_irqsave(&port->lock, flags);
- if (tty->count == 1 && port->count != 1) {
- tty_warn(tty, "%s: tty->count = 1 port count = %d\n", __func__,
- port->count);
- port->count = 1;
- }
- if (--port->count < 0) {
- tty_warn(tty, "%s: bad port count (%d)\n", __func__,
- port->count);
- port->count = 0;
- }
+ scoped_guard(spinlock_irqsave, &port->lock) {
+ if (tty->count == 1 && port->count != 1) {
+ tty_warn(tty, "%s: tty->count = 1 port count = %d\n", __func__,
+ port->count);
+ port->count = 1;
+ }
+ if (--port->count < 0) {
+ tty_warn(tty, "%s: bad port count (%d)\n", __func__,
+ port->count);
+ port->count = 0;
+ }
- if (port->count) {
- spin_unlock_irqrestore(&port->lock, flags);
- return 0;
+ if (port->count)
+ return 0;
}
- spin_unlock_irqrestore(&port->lock, flags);
tty->closing = 1;
@@ -744,9 +729,8 @@ EXPORT_SYMBOL_GPL(tty_port_install);
int tty_port_open(struct tty_port *port, struct tty_struct *tty,
struct file *filp)
{
- spin_lock_irq(&port->lock);
- ++port->count;
- spin_unlock_irq(&port->lock);
+ scoped_guard(spinlock_irq, &port->lock)
+ ++port->count;
tty_port_tty_set(port, tty);
/*
@@ -755,21 +739,17 @@ int tty_port_open(struct tty_port *port, struct tty_struct *tty,
* port mutex.
*/
- mutex_lock(&port->mutex);
-
- if (!tty_port_initialized(port)) {
+ scoped_guard(mutex, &port->mutex) {
+ if (tty_port_initialized(port))
+ break;
clear_bit(TTY_IO_ERROR, &tty->flags);
if (port->ops->activate) {
int retval = port->ops->activate(port, tty);
-
- if (retval) {
- mutex_unlock(&port->mutex);
+ if (retval)
return retval;
- }
}
tty_port_set_initialized(port, true);
}
- mutex_unlock(&port->mutex);
return tty_port_block_til_ready(port, tty, filp);
}
EXPORT_SYMBOL(tty_port_open);
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/16] mxser: use tty_port_tty guard() in mxser_port_isr()
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (4 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 05/16] tty: tty_port: use guard()s Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 07/16] mxser: use guard()s Jiri Slaby (SUSE)
` (9 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Use scoped_guard() and reorder the function. This is done separately
from the other changes, as it is slighly more intrusive: scoped_guard()
handles the have-tty case and returns. The non-tty case is done at the
end of the function.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/mxser.c | 62 +++++++++++++++++++++------------------------
1 file changed, 29 insertions(+), 33 deletions(-)
diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 2fc13cc02cc5..b070ebf9f51a 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -1600,54 +1600,50 @@ static void mxser_transmit_chars(struct tty_struct *tty, struct mxser_port *port
static bool mxser_port_isr(struct mxser_port *port)
{
- struct tty_struct *tty;
u8 iir, status;
- bool error = false;
iir = inb(port->ioaddr + UART_IIR);
if (iir & UART_IIR_NO_INT)
return true;
iir &= MOXA_MUST_IIR_MASK;
- tty = tty_port_tty_get(&port->port);
- if (!tty) {
- status = inb(port->ioaddr + UART_LSR);
- outb(port->FCR | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
- port->ioaddr + UART_FCR);
- inb(port->ioaddr + UART_MSR);
- error = true;
- goto put_tty;
- }
+ scoped_guard(tty_port_tty, &port->port) {
+ struct tty_struct *tty = scoped_tty();
- status = inb(port->ioaddr + UART_LSR);
+ status = inb(port->ioaddr + UART_LSR);
- if (port->board->must_hwid) {
- if (iir == MOXA_MUST_IIR_GDA ||
- iir == MOXA_MUST_IIR_RDA ||
- iir == MOXA_MUST_IIR_RTO ||
- iir == MOXA_MUST_IIR_LSR)
- status = mxser_receive_chars(tty, port, status);
- } else {
- status &= port->read_status_mask;
- if (status & UART_LSR_DR)
- status = mxser_receive_chars(tty, port, status);
- }
+ if (port->board->must_hwid) {
+ if (iir == MOXA_MUST_IIR_GDA ||
+ iir == MOXA_MUST_IIR_RDA ||
+ iir == MOXA_MUST_IIR_RTO ||
+ iir == MOXA_MUST_IIR_LSR)
+ status = mxser_receive_chars(tty, port, status);
+ } else {
+ status &= port->read_status_mask;
+ if (status & UART_LSR_DR)
+ status = mxser_receive_chars(tty, port, status);
+ }
- mxser_check_modem_status(tty, port);
+ mxser_check_modem_status(tty, port);
- if (port->board->must_hwid) {
- if (iir == 0x02 && (status & UART_LSR_THRE))
- mxser_transmit_chars(tty, port);
- } else {
- if (status & UART_LSR_THRE)
- mxser_transmit_chars(tty, port);
+ if (port->board->must_hwid) {
+ if (iir == 0x02 && (status & UART_LSR_THRE))
+ mxser_transmit_chars(tty, port);
+ } else {
+ if (status & UART_LSR_THRE)
+ mxser_transmit_chars(tty, port);
+ }
+
+ return false;
}
-put_tty:
- tty_kref_put(tty);
+ status = inb(port->ioaddr + UART_LSR);
+ outb(port->FCR | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
+ port->ioaddr + UART_FCR);
+ inb(port->ioaddr + UART_MSR);
- return error;
+ return true;
}
/*
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 07/16] mxser: use guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (5 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 06/16] mxser: use tty_port_tty guard() in mxser_port_isr() Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 08/16] serial: serial_core: " Jiri Slaby (SUSE)
` (8 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having all the new guards, use them in the mxser code. This improves
readability, makes error handling easier, and marks locked portions of
code explicit.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/mxser.c | 197 +++++++++++++++++---------------------------
1 file changed, 74 insertions(+), 123 deletions(-)
diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index b070ebf9f51a..94677fec685e 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -442,11 +442,8 @@ static void __mxser_start_tx(struct mxser_port *info)
static void mxser_start_tx(struct mxser_port *info)
{
- unsigned long flags;
-
- spin_lock_irqsave(&info->slock, flags);
+ guard(spinlock_irqsave)(&info->slock);
__mxser_start_tx(info);
- spin_unlock_irqrestore(&info->slock, flags);
}
static void __mxser_stop_tx(struct mxser_port *info)
@@ -465,17 +462,15 @@ static bool mxser_carrier_raised(struct tty_port *port)
static void mxser_dtr_rts(struct tty_port *port, bool active)
{
struct mxser_port *mp = container_of(port, struct mxser_port, port);
- unsigned long flags;
u8 mcr;
- spin_lock_irqsave(&mp->slock, flags);
+ guard(spinlock_irqsave)(&mp->slock);
mcr = inb(mp->ioaddr + UART_MCR);
if (active)
mcr |= UART_MCR_DTR | UART_MCR_RTS;
else
mcr &= ~(UART_MCR_DTR | UART_MCR_RTS);
outb(mcr, mp->ioaddr + UART_MCR);
- spin_unlock_irqrestore(&mp->slock, flags);
}
static int mxser_set_baud(struct tty_struct *tty, speed_t newspd)
@@ -828,32 +823,28 @@ static void mxser_stop_rx(struct mxser_port *info)
static void mxser_shutdown_port(struct tty_port *port)
{
struct mxser_port *info = container_of(port, struct mxser_port, port);
- unsigned long flags;
- spin_lock_irqsave(&info->slock, flags);
-
- mxser_stop_rx(info);
-
- /*
- * clear delta_msr_wait queue to avoid mem leaks: we may free the irq
- * here so the queue might never be waken up
- */
- wake_up_interruptible(&info->port.delta_msr_wait);
+ scoped_guard(spinlock_irqsave, &info->slock) {
+ mxser_stop_rx(info);
- info->IER = 0;
- outb(0x00, info->ioaddr + UART_IER);
-
- /* clear Rx/Tx FIFO's */
- mxser_disable_and_clear_FIFO(info);
+ /*
+ * clear delta_msr_wait queue to avoid mem leaks: we may free the irq
+ * here so the queue might never be waken up
+ */
+ wake_up_interruptible(&info->port.delta_msr_wait);
- /* read data port to reset things */
- (void) inb(info->ioaddr + UART_RX);
+ info->IER = 0;
+ outb(0x00, info->ioaddr + UART_IER);
+ /* clear Rx/Tx FIFO's */
+ mxser_disable_and_clear_FIFO(info);
- if (info->board->must_hwid)
- mxser_must_no_sw_flow_control(info->ioaddr);
+ /* read data port to reset things */
+ (void)inb(info->ioaddr + UART_RX);
- spin_unlock_irqrestore(&info->slock, flags);
+ if (info->board->must_hwid)
+ mxser_must_no_sw_flow_control(info->ioaddr);
+ }
/* make sure ISR is not running while we free the buffer */
synchronize_irq(info->board->irq);
@@ -880,15 +871,13 @@ static int mxser_open(struct tty_struct *tty, struct file *filp)
static void mxser_flush_buffer(struct tty_struct *tty)
{
struct mxser_port *info = tty->driver_data;
- unsigned long flags;
- spin_lock_irqsave(&info->slock, flags);
- kfifo_reset(&info->port.xmit_fifo);
+ scoped_guard(spinlock_irqsave, &info->slock) {
+ kfifo_reset(&info->port.xmit_fifo);
- outb(info->FCR | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
- info->ioaddr + UART_FCR);
-
- spin_unlock_irqrestore(&info->slock, flags);
+ outb(info->FCR | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
+ info->ioaddr + UART_FCR);
+ }
tty_wakeup(tty);
}
@@ -901,14 +890,13 @@ static void mxser_close(struct tty_struct *tty, struct file *filp)
static ssize_t mxser_write(struct tty_struct *tty, const u8 *buf, size_t count)
{
struct mxser_port *info = tty->driver_data;
- unsigned long flags;
size_t written;
bool is_empty;
- spin_lock_irqsave(&info->slock, flags);
- written = kfifo_in(&info->port.xmit_fifo, buf, count);
- is_empty = kfifo_is_empty(&info->port.xmit_fifo);
- spin_unlock_irqrestore(&info->slock, flags);
+ scoped_guard(spinlock_irqsave, &info->slock) {
+ written = kfifo_in(&info->port.xmit_fifo, buf, count);
+ is_empty = kfifo_is_empty(&info->port.xmit_fifo);
+ }
if (!is_empty && !tty->flow.stopped)
if (!tty->hw_stopped || mxser_16550A_or_MUST(info))
@@ -920,14 +908,9 @@ static ssize_t mxser_write(struct tty_struct *tty, const u8 *buf, size_t count)
static int mxser_put_char(struct tty_struct *tty, u8 ch)
{
struct mxser_port *info = tty->driver_data;
- unsigned long flags;
- int ret;
-
- spin_lock_irqsave(&info->slock, flags);
- ret = kfifo_put(&info->port.xmit_fifo, ch);
- spin_unlock_irqrestore(&info->slock, flags);
- return ret;
+ guard(spinlock_irqsave)(&info->slock);
+ return kfifo_put(&info->port.xmit_fifo, ch);
}
@@ -968,7 +951,7 @@ static int mxser_get_serial_info(struct tty_struct *tty,
struct tty_port *port = &info->port;
unsigned int closing_wait, close_delay;
- mutex_lock(&port->mutex);
+ guard(mutex)(&port->mutex);
close_delay = jiffies_to_msecs(info->port.close_delay) / 10;
closing_wait = info->port.closing_wait;
@@ -984,7 +967,7 @@ static int mxser_get_serial_info(struct tty_struct *tty,
ss->close_delay = close_delay;
ss->closing_wait = closing_wait;
ss->custom_divisor = MXSER_CUSTOM_DIVISOR;
- mutex_unlock(&port->mutex);
+
return 0;
}
@@ -994,20 +977,15 @@ static int mxser_set_serial_info(struct tty_struct *tty,
struct mxser_port *info = tty->driver_data;
struct tty_port *port = &info->port;
speed_t baud;
- unsigned long sl_flags;
unsigned int old_speed, close_delay, closing_wait;
- int retval = 0;
if (tty_io_error(tty))
return -EIO;
- mutex_lock(&port->mutex);
+ guard(mutex)(&port->mutex);
- if (ss->irq != info->board->irq ||
- ss->port != info->ioaddr) {
- mutex_unlock(&port->mutex);
+ if (ss->irq != info->board->irq || ss->port != info->ioaddr)
return -EINVAL;
- }
old_speed = port->flags & ASYNC_SPD_MASK;
@@ -1020,10 +998,9 @@ static int mxser_set_serial_info(struct tty_struct *tty,
if ((ss->baud_base != MXSER_BAUD_BASE) ||
(close_delay != port->close_delay) ||
(closing_wait != port->closing_wait) ||
- ((ss->flags & ~ASYNC_USR_MASK) != (port->flags & ~ASYNC_USR_MASK))) {
- mutex_unlock(&port->mutex);
+ ((ss->flags & ~ASYNC_USR_MASK) != (port->flags & ~ASYNC_USR_MASK)))
return -EPERM;
- }
+
port->flags = (port->flags & ~ASYNC_USR_MASK) |
(ss->flags & ASYNC_USR_MASK);
} else {
@@ -1039,10 +1016,9 @@ static int mxser_set_serial_info(struct tty_struct *tty,
(ss->baud_base != MXSER_BAUD_BASE ||
ss->custom_divisor !=
MXSER_CUSTOM_DIVISOR)) {
- if (ss->custom_divisor == 0) {
- mutex_unlock(&port->mutex);
+ if (ss->custom_divisor == 0)
return -EINVAL;
- }
+
baud = ss->baud_base / ss->custom_divisor;
tty_encode_baud_rate(tty, baud, baud);
}
@@ -1054,16 +1030,17 @@ static int mxser_set_serial_info(struct tty_struct *tty,
if (tty_port_initialized(port)) {
if (old_speed != (port->flags & ASYNC_SPD_MASK)) {
- spin_lock_irqsave(&info->slock, sl_flags);
+ guard(spinlock_irqsave)(&info->slock);
mxser_change_speed(tty, NULL);
- spin_unlock_irqrestore(&info->slock, sl_flags);
}
- } else {
- retval = mxser_activate(port, tty);
- if (retval == 0)
- tty_port_set_initialized(port, true);
+
+ return 0;
}
- mutex_unlock(&port->mutex);
+
+ int retval = mxser_activate(port, tty);
+ if (retval == 0)
+ tty_port_set_initialized(port, true);
+
return retval;
}
@@ -1080,13 +1057,11 @@ static int mxser_set_serial_info(struct tty_struct *tty,
static int mxser_get_lsr_info(struct mxser_port *info,
unsigned int __user *value)
{
- unsigned char status;
unsigned int result;
- unsigned long flags;
+ u8 status;
- spin_lock_irqsave(&info->slock, flags);
- status = inb(info->ioaddr + UART_LSR);
- spin_unlock_irqrestore(&info->slock, flags);
+ scoped_guard(spinlock_irqsave, &info->slock)
+ status = inb(info->ioaddr + UART_LSR);
result = ((status & UART_LSR_TEMT) ? TIOCSER_TEMT : 0);
return put_user(result, value);
}
@@ -1095,16 +1070,15 @@ static int mxser_tiocmget(struct tty_struct *tty)
{
struct mxser_port *info = tty->driver_data;
unsigned char control;
- unsigned long flags;
u8 msr;
if (tty_io_error(tty))
return -EIO;
- spin_lock_irqsave(&info->slock, flags);
- control = info->MCR;
- msr = mxser_check_modem_status(tty, info);
- spin_unlock_irqrestore(&info->slock, flags);
+ scoped_guard(spinlock_irqsave, &info->slock) {
+ control = info->MCR;
+ msr = mxser_check_modem_status(tty, info);
+ }
return ((control & UART_MCR_RTS) ? TIOCM_RTS : 0) |
((control & UART_MCR_DTR) ? TIOCM_DTR : 0) |
@@ -1118,12 +1092,11 @@ static int mxser_tiocmset(struct tty_struct *tty,
unsigned int set, unsigned int clear)
{
struct mxser_port *info = tty->driver_data;
- unsigned long flags;
if (tty_io_error(tty))
return -EIO;
- spin_lock_irqsave(&info->slock, flags);
+ guard(spinlock_irqsave)(&info->slock);
if (set & TIOCM_RTS)
info->MCR |= UART_MCR_RTS;
@@ -1136,7 +1109,7 @@ static int mxser_tiocmset(struct tty_struct *tty,
info->MCR &= ~UART_MCR_DTR;
outb(info->MCR, info->ioaddr + UART_MCR);
- spin_unlock_irqrestore(&info->slock, flags);
+
return 0;
}
@@ -1144,12 +1117,11 @@ static int mxser_cflags_changed(struct mxser_port *info, unsigned long arg,
struct async_icount *cprev)
{
struct async_icount cnow;
- unsigned long flags;
int ret;
- spin_lock_irqsave(&info->slock, flags);
- cnow = info->icount; /* atomic copy */
- spin_unlock_irqrestore(&info->slock, flags);
+ /* atomic copy */
+ scoped_guard(spinlock_irqsave, &info->slock)
+ cnow = info->icount;
ret = ((arg & TIOCM_RNG) && (cnow.rng != cprev->rng)) ||
((arg & TIOCM_DSR) && (cnow.dsr != cprev->dsr)) ||
@@ -1179,19 +1151,17 @@ static int mxser_ioctl_op_mode(struct mxser_port *port, int index, bool set,
if (opmode & ~OP_MODE_MASK)
return -EINVAL;
- spin_lock_irq(&port->slock);
+ guard(spinlock_irq)(&port->slock);
val = inb(port->opmode_ioaddr);
val &= ~(OP_MODE_MASK << shiftbit);
val |= (opmode << shiftbit);
outb(val, port->opmode_ioaddr);
- spin_unlock_irq(&port->slock);
return 0;
}
- spin_lock_irq(&port->slock);
- opmode = inb(port->opmode_ioaddr) >> shiftbit;
- spin_unlock_irq(&port->slock);
+ scoped_guard(spinlock_irq, &port->slock)
+ opmode = inb(port->opmode_ioaddr) >> shiftbit;
return put_user(opmode & OP_MODE_MASK, u_opmode);
}
@@ -1201,7 +1171,6 @@ static int mxser_ioctl(struct tty_struct *tty,
{
struct mxser_port *info = tty->driver_data;
struct async_icount cnow;
- unsigned long flags;
void __user *argp = (void __user *)arg;
if (cmd == MOXA_SET_OP_MODE || cmd == MOXA_GET_OP_MODE)
@@ -1221,9 +1190,9 @@ static int mxser_ioctl(struct tty_struct *tty,
* Caller should use TIOCGICOUNT to see which one it was
*/
case TIOCMIWAIT:
- spin_lock_irqsave(&info->slock, flags);
- cnow = info->icount; /* note the counters on entry */
- spin_unlock_irqrestore(&info->slock, flags);
+ /* note the counters on entry */
+ scoped_guard(spinlock_irqsave, &info->slock)
+ cnow = info->icount;
return wait_event_interruptible(info->port.delta_msr_wait,
mxser_cflags_changed(info, arg, &cnow));
@@ -1246,11 +1215,9 @@ static int mxser_get_icount(struct tty_struct *tty,
{
struct mxser_port *info = tty->driver_data;
struct async_icount cnow;
- unsigned long flags;
- spin_lock_irqsave(&info->slock, flags);
- cnow = info->icount;
- spin_unlock_irqrestore(&info->slock, flags);
+ scoped_guard(spinlock_irqsave, &info->slock)
+ cnow = info->icount;
icount->frame = cnow.frame;
icount->brk = cnow.brk;
@@ -1328,34 +1295,28 @@ static void mxser_unthrottle(struct tty_struct *tty)
static void mxser_stop(struct tty_struct *tty)
{
struct mxser_port *info = tty->driver_data;
- unsigned long flags;
- spin_lock_irqsave(&info->slock, flags);
+ guard(spinlock_irqsave)(&info->slock);
if (info->IER & UART_IER_THRI)
__mxser_stop_tx(info);
- spin_unlock_irqrestore(&info->slock, flags);
}
static void mxser_start(struct tty_struct *tty)
{
struct mxser_port *info = tty->driver_data;
- unsigned long flags;
- spin_lock_irqsave(&info->slock, flags);
+ guard(spinlock_irqsave)(&info->slock);
if (!kfifo_is_empty(&info->port.xmit_fifo))
__mxser_start_tx(info);
- spin_unlock_irqrestore(&info->slock, flags);
}
static void mxser_set_termios(struct tty_struct *tty,
const struct ktermios *old_termios)
{
struct mxser_port *info = tty->driver_data;
- unsigned long flags;
- spin_lock_irqsave(&info->slock, flags);
- mxser_change_speed(tty, old_termios);
- spin_unlock_irqrestore(&info->slock, flags);
+ scoped_guard(spinlock_irqsave, &info->slock)
+ mxser_change_speed(tty, old_termios);
if ((old_termios->c_cflag & CRTSCTS) && !C_CRTSCTS(tty)) {
tty->hw_stopped = false;
@@ -1367,9 +1328,8 @@ static void mxser_set_termios(struct tty_struct *tty,
tty->flow.stopped = 0;
if (info->board->must_hwid) {
- spin_lock_irqsave(&info->slock, flags);
+ guard(spinlock_irqsave)(&info->slock);
mxser_must_set_rx_sw_flow_control(info->ioaddr, false);
- spin_unlock_irqrestore(&info->slock, flags);
}
mxser_start(tty);
@@ -1378,14 +1338,8 @@ static void mxser_set_termios(struct tty_struct *tty,
static bool mxser_tx_empty(struct mxser_port *info)
{
- unsigned long flags;
- u8 lsr;
-
- spin_lock_irqsave(&info->slock, flags);
- lsr = inb(info->ioaddr + UART_LSR);
- spin_unlock_irqrestore(&info->slock, flags);
-
- return !(lsr & UART_LSR_TEMT);
+ guard(spinlock_irqsave)(&info->slock);
+ return !(inb(info->ioaddr + UART_LSR) & UART_LSR_TEMT);
}
/*
@@ -1459,17 +1413,15 @@ static void mxser_hangup(struct tty_struct *tty)
static int mxser_rs_break(struct tty_struct *tty, int break_state)
{
struct mxser_port *info = tty->driver_data;
- unsigned long flags;
u8 lcr;
- spin_lock_irqsave(&info->slock, flags);
+ guard(spinlock_irqsave)(&info->slock);
lcr = inb(info->ioaddr + UART_LCR);
if (break_state == -1)
lcr |= UART_LCR_SBC;
else
lcr &= ~UART_LCR_SBC;
outb(lcr, info->ioaddr + UART_LCR);
- spin_unlock_irqrestore(&info->slock, flags);
return 0;
}
@@ -1672,12 +1624,11 @@ static irqreturn_t mxser_interrupt(int irq, void *dev_id)
port = &brd->ports[i];
int_cnt = 0;
- spin_lock(&port->slock);
+ guard(spinlock)(&port->slock);
do {
if (mxser_port_isr(port))
break;
} while (int_cnt++ < MXSER_ISR_PASS_LIMIT);
- spin_unlock(&port->slock);
}
}
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 08/16] serial: serial_core: use guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (6 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 07/16] mxser: use guard()s Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 09/16] serial: 8250: " Jiri Slaby (SUSE)
` (7 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having all the new guards, use them in the serial_core code. This
improves readability, makes error handling easier, and marks locked
portions of code explicit.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/serial/serial_core.c | 143 +++++++++++++------------------
1 file changed, 59 insertions(+), 84 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 86d404d649a3..4757293ece8c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -177,15 +177,13 @@ static void uart_start(struct tty_struct *tty)
static void
uart_update_mctrl(struct uart_port *port, unsigned int set, unsigned int clear)
{
- unsigned long flags;
unsigned int old;
- uart_port_lock_irqsave(port, &flags);
+ guard(uart_port_lock_irqsave)(port);
old = port->mctrl;
port->mctrl = (old & ~clear) | set;
if (old != port->mctrl && !(port->rs485.flags & SER_RS485_ENABLED))
port->ops->set_mctrl(port, port->mctrl);
- uart_port_unlock_irqrestore(port, flags);
}
#define uart_set_mctrl(port, set) uart_update_mctrl(port, set, 0)
@@ -220,7 +218,7 @@ static void uart_change_line_settings(struct tty_struct *tty, struct uart_state
/*
* Set modem status enables based on termios cflag
*/
- uart_port_lock_irq(uport);
+ guard(uart_port_lock_irq)(uport);
if (termios->c_cflag & CRTSCTS)
uport->status |= UPSTAT_CTS_ENABLE;
else
@@ -241,7 +239,6 @@ static void uart_change_line_settings(struct tty_struct *tty, struct uart_state
else
__uart_start(state);
}
- uart_port_unlock_irq(uport);
}
static int uart_alloc_xmit_buf(struct tty_port *port)
@@ -711,7 +708,6 @@ static void uart_send_xchar(struct tty_struct *tty, u8 ch)
{
struct uart_state *state = tty->driver_data;
struct uart_port *port;
- unsigned long flags;
port = uart_port_ref(state);
if (!port)
@@ -720,11 +716,10 @@ static void uart_send_xchar(struct tty_struct *tty, u8 ch)
if (port->ops->send_xchar)
port->ops->send_xchar(port, ch);
else {
- uart_port_lock_irqsave(port, &flags);
+ guard(uart_port_lock_irqsave)(port);
port->x_char = ch;
if (ch)
port->ops->start_tx(port);
- uart_port_unlock_irqrestore(port, flags);
}
uart_port_deref(port);
}
@@ -1089,7 +1084,6 @@ static int uart_tiocmget(struct tty_struct *tty)
struct uart_state *state = tty->driver_data;
struct tty_port *port = &state->port;
struct uart_port *uport;
- int result;
guard(mutex)(&port->mutex);
@@ -1097,12 +1091,9 @@ static int uart_tiocmget(struct tty_struct *tty)
if (!uport || tty_io_error(tty))
return -EIO;
- uart_port_lock_irq(uport);
- result = uport->mctrl;
- result |= uport->ops->get_mctrl(uport);
- uart_port_unlock_irq(uport);
+ guard(uart_port_lock_irq)(uport);
- return result;
+ return uport->mctrl | uport->ops->get_mctrl(uport);
}
static int
@@ -1226,16 +1217,15 @@ static int uart_wait_modem_status(struct uart_state *state, unsigned long arg)
uport = uart_port_ref(state);
if (!uport)
return -EIO;
- uart_port_lock_irq(uport);
- memcpy(&cprev, &uport->icount, sizeof(struct uart_icount));
- uart_enable_ms(uport);
- uart_port_unlock_irq(uport);
+ scoped_guard(uart_port_lock_irq, uport) {
+ memcpy(&cprev, &uport->icount, sizeof(struct uart_icount));
+ uart_enable_ms(uport);
+ }
add_wait_queue(&port->delta_msr_wait, &wait);
for (;;) {
- uart_port_lock_irq(uport);
- memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
- uart_port_unlock_irq(uport);
+ scoped_guard(uart_port_lock_irq, uport)
+ memcpy(&cnow, &uport->icount, sizeof(struct uart_icount));
set_current_state(TASK_INTERRUPTIBLE);
@@ -1430,7 +1420,6 @@ static void uart_set_rs485_rx_during_tx(struct uart_port *port,
static int uart_rs485_config(struct uart_port *port)
{
struct serial_rs485 *rs485 = &port->rs485;
- unsigned long flags;
int ret;
if (!(rs485->flags & SER_RS485_ENABLED))
@@ -1440,9 +1429,8 @@ static int uart_rs485_config(struct uart_port *port)
uart_set_rs485_termination(port, rs485);
uart_set_rs485_rx_during_tx(port, rs485);
- uart_port_lock_irqsave(port, &flags);
- ret = port->rs485_config(port, NULL, rs485);
- uart_port_unlock_irqrestore(port, flags);
+ scoped_guard(uart_port_lock_irqsave, port)
+ ret = port->rs485_config(port, NULL, rs485);
if (ret) {
memset(rs485, 0, sizeof(*rs485));
/* unset GPIOs */
@@ -1456,12 +1444,10 @@ static int uart_rs485_config(struct uart_port *port)
static int uart_get_rs485_config(struct uart_port *port,
struct serial_rs485 __user *rs485)
{
- unsigned long flags;
struct serial_rs485 aux;
- uart_port_lock_irqsave(port, &flags);
- aux = port->rs485;
- uart_port_unlock_irqrestore(port, flags);
+ scoped_guard(uart_port_lock_irqsave, port)
+ aux = port->rs485;
if (copy_to_user(rs485, &aux, sizeof(aux)))
return -EFAULT;
@@ -1474,7 +1460,6 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
{
struct serial_rs485 rs485;
int ret;
- unsigned long flags;
if (!(port->rs485_supported.flags & SER_RS485_ENABLED))
return -ENOTTY;
@@ -1489,16 +1474,16 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
uart_set_rs485_termination(port, &rs485);
uart_set_rs485_rx_during_tx(port, &rs485);
- uart_port_lock_irqsave(port, &flags);
- ret = port->rs485_config(port, &tty->termios, &rs485);
- if (!ret) {
- port->rs485 = rs485;
+ scoped_guard(uart_port_lock_irqsave, port) {
+ ret = port->rs485_config(port, &tty->termios, &rs485);
+ if (!ret) {
+ port->rs485 = rs485;
- /* Reset RTS and other mctrl lines when disabling RS485 */
- if (!(rs485.flags & SER_RS485_ENABLED))
- port->ops->set_mctrl(port, port->mctrl);
+ /* Reset RTS and other mctrl lines when disabling RS485 */
+ if (!(rs485.flags & SER_RS485_ENABLED))
+ port->ops->set_mctrl(port, port->mctrl);
+ }
}
- uart_port_unlock_irqrestore(port, flags);
if (ret) {
/* restore old GPIO settings */
gpiod_set_value_cansleep(port->rs485_term_gpio,
@@ -1517,15 +1502,13 @@ static int uart_set_rs485_config(struct tty_struct *tty, struct uart_port *port,
static int uart_get_iso7816_config(struct uart_port *port,
struct serial_iso7816 __user *iso7816)
{
- unsigned long flags;
struct serial_iso7816 aux;
if (!port->iso7816_config)
return -ENOTTY;
- uart_port_lock_irqsave(port, &flags);
- aux = port->iso7816;
- uart_port_unlock_irqrestore(port, flags);
+ scoped_guard(uart_port_lock_irqsave, port)
+ aux = port->iso7816;
if (copy_to_user(iso7816, &aux, sizeof(aux)))
return -EFAULT;
@@ -1537,8 +1520,7 @@ static int uart_set_iso7816_config(struct uart_port *port,
struct serial_iso7816 __user *iso7816_user)
{
struct serial_iso7816 iso7816;
- int i, ret;
- unsigned long flags;
+ int i;
if (!port->iso7816_config)
return -ENOTTY;
@@ -1554,11 +1536,11 @@ static int uart_set_iso7816_config(struct uart_port *port,
if (iso7816.reserved[i])
return -EINVAL;
- uart_port_lock_irqsave(port, &flags);
- ret = port->iso7816_config(port, &iso7816);
- uart_port_unlock_irqrestore(port, flags);
- if (ret)
- return ret;
+ scoped_guard(uart_port_lock_irqsave, port) {
+ int ret = port->iso7816_config(port, &iso7816);
+ if (ret)
+ return ret;
+ }
if (copy_to_user(iso7816_user, &port->iso7816, sizeof(port->iso7816)))
return -EFAULT;
@@ -1770,9 +1752,8 @@ static void uart_tty_port_shutdown(struct tty_port *port)
if (WARN(!uport, "detached port still initialized!\n"))
return;
- uart_port_lock_irq(uport);
- uport->ops->stop_rx(uport);
- uart_port_unlock_irq(uport);
+ scoped_guard(uart_port_lock_irq, uport)
+ uport->ops->stop_rx(uport);
serial_base_port_shutdown(uport);
uart_port_shutdown(port);
@@ -2044,9 +2025,8 @@ static void uart_line_info(struct seq_file *m, struct uart_state *state)
pm_state = state->pm_state;
if (pm_state != UART_PM_STATE_ON)
uart_change_pm(state, UART_PM_STATE_ON);
- uart_port_lock_irq(uport);
- status = uport->ops->get_mctrl(uport);
- uart_port_unlock_irq(uport);
+ scoped_guard(uart_port_lock_irq, uport)
+ status = uport->ops->get_mctrl(uport);
if (pm_state != UART_PM_STATE_ON)
uart_change_pm(state, pm_state);
@@ -2355,9 +2335,8 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
*/
if (!console_suspend_enabled && uart_console(uport)) {
if (uport->ops->start_rx) {
- uart_port_lock_irq(uport);
+ guard(uart_port_lock_irq)(uport);
uport->ops->stop_rx(uport);
- uart_port_unlock_irq(uport);
}
device_set_awake_path(uport->dev);
return 0;
@@ -2373,15 +2352,15 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
tty_port_set_suspended(port, true);
tty_port_set_initialized(port, false);
- uart_port_lock_irq(uport);
- ops->stop_tx(uport);
- if (!(uport->rs485.flags & SER_RS485_ENABLED))
- ops->set_mctrl(uport, 0);
- /* save mctrl so it can be restored on resume */
- mctrl = uport->mctrl;
- uport->mctrl = 0;
- ops->stop_rx(uport);
- uart_port_unlock_irq(uport);
+ scoped_guard(uart_port_lock_irq, uport) {
+ ops->stop_tx(uport);
+ if (!(uport->rs485.flags & SER_RS485_ENABLED))
+ ops->set_mctrl(uport, 0);
+ /* save mctrl so it can be restored on resume */
+ mctrl = uport->mctrl;
+ uport->mctrl = 0;
+ ops->stop_rx(uport);
+ }
/*
* Wait for the transmitter to empty.
@@ -2450,9 +2429,8 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
uart_change_pm(state, UART_PM_STATE_ON);
uport->ops->set_termios(uport, &termios, NULL);
if (!console_suspend_enabled && uport->ops->start_rx) {
- uart_port_lock_irq(uport);
+ guard(uart_port_lock_irq)(uport);
uport->ops->start_rx(uport);
- uart_port_unlock_irq(uport);
}
if (console_suspend_enabled)
console_resume(uport->cons);
@@ -2463,10 +2441,9 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
int ret;
uart_change_pm(state, UART_PM_STATE_ON);
- uart_port_lock_irq(uport);
- if (!(uport->rs485.flags & SER_RS485_ENABLED))
- ops->set_mctrl(uport, 0);
- uart_port_unlock_irq(uport);
+ scoped_guard(uart_port_lock_irq, uport)
+ if (!(uport->rs485.flags & SER_RS485_ENABLED))
+ ops->set_mctrl(uport, 0);
if (console_suspend_enabled || !uart_console(uport)) {
/* Protected by port mutex for now */
struct tty_struct *tty = port->tty;
@@ -2476,11 +2453,11 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
if (tty)
uart_change_line_settings(tty, state, NULL);
uart_rs485_config(uport);
- uart_port_lock_irq(uport);
- if (!(uport->rs485.flags & SER_RS485_ENABLED))
- ops->set_mctrl(uport, uport->mctrl);
- ops->start_tx(uport);
- uart_port_unlock_irq(uport);
+ scoped_guard(uart_port_lock_irq, uport) {
+ if (!(uport->rs485.flags & SER_RS485_ENABLED))
+ ops->set_mctrl(uport, uport->mctrl);
+ ops->start_tx(uport);
+ }
tty_port_set_initialized(port, true);
} else {
/*
@@ -2574,8 +2551,6 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
}
if (port->type != PORT_UNKNOWN) {
- unsigned long flags;
-
uart_report_port(drv, port);
/* Synchronize with possible boot console. */
@@ -2590,11 +2565,11 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
* keep the DTR setting that is set in uart_set_options()
* We probably don't need a spinlock around this, but
*/
- uart_port_lock_irqsave(port, &flags);
- port->mctrl &= TIOCM_DTR;
- if (!(port->rs485.flags & SER_RS485_ENABLED))
- port->ops->set_mctrl(port, port->mctrl);
- uart_port_unlock_irqrestore(port, flags);
+ scoped_guard(uart_port_lock_irqsave, port) {
+ port->mctrl &= TIOCM_DTR;
+ if (!(port->rs485.flags & SER_RS485_ENABLED))
+ port->ops->set_mctrl(port, port->mctrl);
+ }
uart_rs485_config(port);
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 09/16] serial: 8250: use guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (7 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 08/16] serial: serial_core: " Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 10/16] serial: 8250_core: use guard() in serial_unlink_irq_chain() Jiri Slaby (SUSE)
` (6 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having all the new guards, use them in the 8250 code. This improves
readability, makes error handling easier, and marks locked portions of
code explicit.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/serial/8250/8250_core.c | 71 +++----
drivers/tty/serial/8250/8250_port.c | 292 +++++++++++-----------------
2 files changed, 143 insertions(+), 220 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index feb920c5b2e8..82c3636451e5 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -72,7 +72,7 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
struct list_head *l, *end = NULL;
int pass_counter = 0, handled = 0;
- spin_lock(&i->lock);
+ guard(spinlock)(&i->lock);
l = i->head;
do {
@@ -91,8 +91,6 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
break;
} while (l != end);
- spin_unlock(&i->lock);
-
return IRQ_RETVAL(handled);
}
@@ -132,22 +130,19 @@ static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_por
{
struct irq_info *i;
- mutex_lock(&hash_mutex);
+ guard(mutex)(&hash_mutex);
hash_for_each_possible(irq_lists, i, node, up->port.irq)
if (i->irq == up->port.irq)
- goto unlock;
+ return i;
i = kzalloc(sizeof(*i), GFP_KERNEL);
- if (i == NULL) {
- i = ERR_PTR(-ENOMEM);
- goto unlock;
- }
+ if (i == NULL)
+ return ERR_PTR(-ENOMEM);
+
spin_lock_init(&i->lock);
i->irq = up->port.irq;
hash_add(irq_lists, &i->node, i->irq);
-unlock:
- mutex_unlock(&hash_mutex);
return i;
}
@@ -161,23 +156,21 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
if (IS_ERR(i))
return PTR_ERR(i);
- spin_lock_irq(&i->lock);
+ scoped_guard(spinlock_irq, &i->lock) {
+ if (i->head) {
+ list_add(&up->list, i->head);
- if (i->head) {
- list_add(&up->list, i->head);
- spin_unlock_irq(&i->lock);
+ return 0;
+ }
- ret = 0;
- } else {
INIT_LIST_HEAD(&up->list);
i->head = &up->list;
- spin_unlock_irq(&i->lock);
- ret = request_irq(up->port.irq, serial8250_interrupt,
- up->port.irqflags, up->port.name, i);
- if (ret < 0)
- serial_do_unlink(i, up);
}
+ ret = request_irq(up->port.irq, serial8250_interrupt, up->port.irqflags, up->port.name, i);
+ if (ret < 0)
+ serial_do_unlink(i, up);
+
return ret;
}
@@ -670,16 +663,12 @@ static struct uart_8250_port *serial8250_find_match_or_unused(const struct uart_
static void serial_8250_overrun_backoff_work(struct work_struct *work)
{
- struct uart_8250_port *up =
- container_of(to_delayed_work(work), struct uart_8250_port,
- overrun_backoff);
- struct uart_port *port = &up->port;
- unsigned long flags;
+ struct uart_8250_port *up = container_of(to_delayed_work(work), struct uart_8250_port,
+ overrun_backoff);
- uart_port_lock_irqsave(port, &flags);
+ guard(uart_port_lock_irqsave)(&up->port);
up->ier |= UART_IER_RLSI | UART_IER_RDI;
serial_out(up, UART_IER, up->ier);
- uart_port_unlock_irqrestore(port, flags);
}
/**
@@ -698,12 +687,12 @@ static void serial_8250_overrun_backoff_work(struct work_struct *work)
int serial8250_register_8250_port(const struct uart_8250_port *up)
{
struct uart_8250_port *uart;
- int ret = -ENOSPC;
+ int ret;
if (up->port.uartclk == 0)
return -EINVAL;
- mutex_lock(&serial_mutex);
+ guard(mutex)(&serial_mutex);
uart = serial8250_find_match_or_unused(&up->port);
if (!uart) {
@@ -713,15 +702,13 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
*/
uart = serial8250_setup_port(nr_uarts);
if (!uart)
- goto unlock;
+ return -ENOSPC;
nr_uarts++;
}
/* Check if it is CIR already. We check this below again, see there why. */
- if (uart->port.type == PORT_8250_CIR) {
- ret = -ENODEV;
- goto unlock;
- }
+ if (uart->port.type == PORT_8250_CIR)
+ return -ENODEV;
if (uart->port.dev)
uart_remove_one_port(&serial8250_reg, &uart->port);
@@ -855,14 +842,10 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
uart->overrun_backoff_time_ms = 0;
}
-unlock:
- mutex_unlock(&serial_mutex);
-
return ret;
err:
uart->port.dev = NULL;
- mutex_unlock(&serial_mutex);
return ret;
}
EXPORT_SYMBOL(serial8250_register_8250_port);
@@ -878,14 +861,11 @@ void serial8250_unregister_port(int line)
{
struct uart_8250_port *uart = &serial8250_ports[line];
- mutex_lock(&serial_mutex);
+ guard(mutex)(&serial_mutex);
if (uart->em485) {
- unsigned long flags;
-
- uart_port_lock_irqsave(&uart->port, &flags);
+ guard(uart_port_lock_irqsave)(&uart->port);
serial8250_em485_destroy(uart);
- uart_port_unlock_irqrestore(&uart->port, flags);
}
uart_remove_one_port(&serial8250_reg, &uart->port);
@@ -901,7 +881,6 @@ void serial8250_unregister_port(int line)
} else {
uart->port.dev = NULL;
}
- mutex_unlock(&serial_mutex);
}
EXPORT_SYMBOL(serial8250_unregister_port);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 5afae4025696..719faf92aa8a 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -674,28 +674,27 @@ static void serial8250_set_sleep(struct uart_8250_port *p, int sleep)
{
unsigned char lcr = 0, efr = 0;
- serial8250_rpm_get(p);
-
- if (p->capabilities & UART_CAP_SLEEP) {
- /* Synchronize UART_IER access against the console. */
- uart_port_lock_irq(&p->port);
- if (p->capabilities & UART_CAP_EFR) {
- lcr = serial_in(p, UART_LCR);
- efr = serial_in(p, UART_EFR);
- serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B);
- serial_out(p, UART_EFR, UART_EFR_ECB);
- serial_out(p, UART_LCR, 0);
- }
- serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0);
- if (p->capabilities & UART_CAP_EFR) {
- serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B);
- serial_out(p, UART_EFR, efr);
- serial_out(p, UART_LCR, lcr);
- }
- uart_port_unlock_irq(&p->port);
- }
+ guard(serial8250_rpm)(p);
+
+ if (!(p->capabilities & UART_CAP_SLEEP))
+ return;
+
+ /* Synchronize UART_IER access against the console. */
+ guard(uart_port_lock_irq)(&p->port);
- serial8250_rpm_put(p);
+ if (p->capabilities & UART_CAP_EFR) {
+ lcr = serial_in(p, UART_LCR);
+ efr = serial_in(p, UART_EFR);
+ serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B);
+ serial_out(p, UART_EFR, UART_EFR_ECB);
+ serial_out(p, UART_LCR, 0);
+ }
+ serial_out(p, UART_IER, sleep ? UART_IERX_SLEEP : 0);
+ if (p->capabilities & UART_CAP_EFR) {
+ serial_out(p, UART_LCR, UART_LCR_CONF_MODE_B);
+ serial_out(p, UART_EFR, efr);
+ serial_out(p, UART_LCR, lcr);
+ }
}
/* Clear the interrupt registers. */
@@ -1231,9 +1230,8 @@ static void autoconfig_irq(struct uart_8250_port *up)
probe_irq_off(probe_irq_on());
save_mcr = serial8250_in_MCR(up);
/* Synchronize UART_IER access against the console. */
- uart_port_lock_irq(port);
- save_ier = serial_in(up, UART_IER);
- uart_port_unlock_irq(port);
+ scoped_guard(uart_port_lock_irq, port)
+ save_ier = serial_in(up, UART_IER);
serial8250_out_MCR(up, UART_MCR_OUT1 | UART_MCR_OUT2);
irqs = probe_irq_on();
@@ -1246,9 +1244,8 @@ static void autoconfig_irq(struct uart_8250_port *up)
UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2);
}
/* Synchronize UART_IER access against the console. */
- uart_port_lock_irq(port);
- serial_out(up, UART_IER, UART_IER_ALL_INTR);
- uart_port_unlock_irq(port);
+ scoped_guard(uart_port_lock_irq, port)
+ serial_out(up, UART_IER, UART_IER_ALL_INTR);
serial8250_clear_interrupts(port);
serial_out(up, UART_TX, 0xFF);
udelay(20);
@@ -1256,9 +1253,8 @@ static void autoconfig_irq(struct uart_8250_port *up)
serial8250_out_MCR(up, save_mcr);
/* Synchronize UART_IER access against the console. */
- uart_port_lock_irq(port);
- serial_out(up, UART_IER, save_ier);
- uart_port_unlock_irq(port);
+ scoped_guard(uart_port_lock_irq, port)
+ serial_out(up, UART_IER, save_ier);
if (port->flags & UPF_FOURPORT)
outb_p(save_ICP, ICP);
@@ -1273,12 +1269,10 @@ static void serial8250_stop_rx(struct uart_port *port)
/* Port locked to synchronize UART_IER access against the console. */
lockdep_assert_held_once(&port->lock);
- serial8250_rpm_get(up);
+ guard(serial8250_rpm)(up);
up->ier &= ~(UART_IER_RLSI | UART_IER_RDI);
serial_port_out(port, UART_IER, up->ier);
-
- serial8250_rpm_put(up);
}
/**
@@ -1322,17 +1316,15 @@ static enum hrtimer_restart serial8250_em485_handle_stop_tx(struct hrtimer *t)
struct uart_8250_em485 *em485 = container_of(t, struct uart_8250_em485,
stop_tx_timer);
struct uart_8250_port *p = em485->port;
- unsigned long flags;
- serial8250_rpm_get(p);
- uart_port_lock_irqsave(&p->port, &flags);
+ guard(serial8250_rpm)(p);
+ guard(uart_port_lock_irqsave)(&p->port);
+
if (em485->active_timer == &em485->stop_tx_timer) {
p->rs485_stop_tx(p, true);
em485->active_timer = NULL;
em485->tx_stopped = true;
}
- uart_port_unlock_irqrestore(&p->port, flags);
- serial8250_rpm_put(p);
return HRTIMER_NORESTART;
}
@@ -1407,7 +1399,7 @@ static void serial8250_stop_tx(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
- serial8250_rpm_get(up);
+ guard(serial8250_rpm)(up);
__stop_tx(up);
/*
@@ -1417,7 +1409,6 @@ static void serial8250_stop_tx(struct uart_port *port)
up->acr |= UART_ACR_TXDIS;
serial_icr_write(up, UART_ACR, up->acr);
}
- serial8250_rpm_put(up);
}
static inline void __start_tx(struct uart_port *port)
@@ -1512,14 +1503,13 @@ static enum hrtimer_restart serial8250_em485_handle_start_tx(struct hrtimer *t)
struct uart_8250_em485 *em485 = container_of(t, struct uart_8250_em485,
start_tx_timer);
struct uart_8250_port *p = em485->port;
- unsigned long flags;
- uart_port_lock_irqsave(&p->port, &flags);
+ guard(uart_port_lock_irqsave)(&p->port);
+
if (em485->active_timer == &em485->start_tx_timer) {
__start_tx(&p->port);
em485->active_timer = NULL;
}
- uart_port_unlock_irqrestore(&p->port, flags);
return HRTIMER_NORESTART;
}
@@ -1587,9 +1577,8 @@ static void serial8250_enable_ms(struct uart_port *port)
up->ier |= UART_IER_MSI;
- serial8250_rpm_get(up);
+ guard(serial8250_rpm)(up);
serial_port_out(port, UART_IER, up->ier);
- serial8250_rpm_put(up);
}
void serial8250_read_char(struct uart_8250_port *up, u16 lsr)
@@ -1850,15 +1839,11 @@ static int serial8250_default_handle_irq(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
unsigned int iir;
- int ret;
- serial8250_rpm_get(up);
+ guard(serial8250_rpm)(up);
iir = serial_port_in(port, UART_IIR);
- ret = serial8250_handle_irq(port, iir);
-
- serial8250_rpm_put(up);
- return ret;
+ return serial8250_handle_irq(port, iir);
}
/*
@@ -1869,16 +1854,14 @@ static int serial8250_default_handle_irq(struct uart_port *port)
*/
static int serial8250_tx_threshold_handle_irq(struct uart_port *port)
{
- unsigned long flags;
unsigned int iir = serial_port_in(port, UART_IIR);
/* TX Threshold IRQ triggered so load up FIFO */
if ((iir & UART_IIR_ID) == UART_IIR_THRI) {
struct uart_8250_port *up = up_to_u8250p(port);
- uart_port_lock_irqsave(port, &flags);
+ guard(uart_port_lock_irqsave)(port);
serial8250_tx_chars(up);
- uart_port_unlock_irqrestore(port, flags);
}
iir = serial_port_in(port, UART_IIR);
@@ -1888,19 +1871,14 @@ static int serial8250_tx_threshold_handle_irq(struct uart_port *port)
static unsigned int serial8250_tx_empty(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned int result = 0;
- unsigned long flags;
- serial8250_rpm_get(up);
+ guard(serial8250_rpm)(up);
+ guard(uart_port_lock_irqsave)(port);
- uart_port_lock_irqsave(port, &flags);
if (!serial8250_tx_dma_running(up) && uart_lsr_tx_empty(serial_lsr_in(up)))
- result = TIOCSER_TEMT;
- uart_port_unlock_irqrestore(port, flags);
-
- serial8250_rpm_put(up);
+ return TIOCSER_TEMT;
- return result;
+ return 0;
}
unsigned int serial8250_do_get_mctrl(struct uart_port *port)
@@ -1909,9 +1887,8 @@ unsigned int serial8250_do_get_mctrl(struct uart_port *port)
unsigned int status;
unsigned int val;
- serial8250_rpm_get(up);
- status = serial8250_modem_status(up);
- serial8250_rpm_put(up);
+ scoped_guard(serial8250_rpm, up)
+ status = serial8250_modem_status(up);
val = serial8250_MSR_to_TIOCM(status);
if (up->gpios)
@@ -1955,17 +1932,15 @@ static void serial8250_set_mctrl(struct uart_port *port, unsigned int mctrl)
static void serial8250_break_ctl(struct uart_port *port, int break_state)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned long flags;
- serial8250_rpm_get(up);
- uart_port_lock_irqsave(port, &flags);
+ guard(serial8250_rpm)(up);
+ guard(uart_port_lock_irqsave)(port);
+
if (break_state == -1)
up->lcr |= UART_LCR_SBC;
else
up->lcr &= ~UART_LCR_SBC;
serial_port_out(port, UART_LCR, up->lcr);
- uart_port_unlock_irqrestore(port, flags);
- serial8250_rpm_put(up);
}
/* Returns true if @bits were set, false on timeout */
@@ -2025,22 +2000,15 @@ static void wait_for_xmitr(struct uart_8250_port *up, int bits)
static int serial8250_get_poll_char(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
- int status;
u16 lsr;
- serial8250_rpm_get(up);
+ guard(serial8250_rpm)(up);
lsr = serial_port_in(port, UART_LSR);
+ if (!(lsr & UART_LSR_DR))
+ return NO_POLL_CHAR;
- if (!(lsr & UART_LSR_DR)) {
- status = NO_POLL_CHAR;
- goto out;
- }
-
- status = serial_port_in(port, UART_RX);
-out:
- serial8250_rpm_put(up);
- return status;
+ return serial_port_in(port, UART_RX);
}
@@ -2058,7 +2026,7 @@ static void serial8250_put_poll_char(struct uart_port *port,
* should allow safe lockless usage here.
*/
- serial8250_rpm_get(up);
+ guard(serial8250_rpm)(up);
/*
* First save the IER then disable the interrupts
*/
@@ -2077,7 +2045,6 @@ static void serial8250_put_poll_char(struct uart_port *port,
*/
wait_for_xmitr(up, UART_LSR_BOTH_EMPTY);
serial_port_out(port, UART_IER, ier);
- serial8250_rpm_put(up);
}
#endif /* CONFIG_CONSOLE_POLL */
@@ -2085,16 +2052,15 @@ static void serial8250_put_poll_char(struct uart_port *port,
static void serial8250_startup_special(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned long flags;
switch (port->type) {
- case PORT_16C950:
+ case PORT_16C950: {
/*
* Wake up and initialize UART
*
* Synchronize UART_IER access against the console.
*/
- uart_port_lock_irqsave(port, &flags);
+ guard(uart_port_lock_irqsave)(port);
up->acr = 0;
serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B);
serial_port_out(port, UART_EFR, UART_EFR_ECB);
@@ -2104,18 +2070,18 @@ static void serial8250_startup_special(struct uart_port *port)
serial_port_out(port, UART_LCR, UART_LCR_CONF_MODE_B);
serial_port_out(port, UART_EFR, UART_EFR_ECB);
serial_port_out(port, UART_LCR, 0);
- uart_port_unlock_irqrestore(port, flags);
break;
+ }
case PORT_DA830:
/*
* Reset the port
*
* Synchronize UART_IER access against the console.
*/
- uart_port_lock_irqsave(port, &flags);
- serial_port_out(port, UART_IER, 0);
- serial_port_out(port, UART_DA830_PWREMU_MGMT, 0);
- uart_port_unlock_irqrestore(port, flags);
+ scoped_guard(uart_port_lock_irqsave, port) {
+ serial_port_out(port, UART_IER, 0);
+ serial_port_out(port, UART_DA830_PWREMU_MGMT, 0);
+ }
mdelay(10);
/* Enable Tx, Rx and free run mode */
@@ -2173,7 +2139,6 @@ static void serial8250_set_TRG_levels(struct uart_port *port)
static void serial8250_THRE_test(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned long flags;
bool iir_noint1, iir_noint2;
if (!port->irq)
@@ -2193,19 +2158,17 @@ static void serial8250_THRE_test(struct uart_port *port)
*
* Synchronize UART_IER access against the console.
*/
- uart_port_lock_irqsave(port, &flags);
-
- wait_for_xmitr(up, UART_LSR_THRE);
- serial_port_out_sync(port, UART_IER, UART_IER_THRI);
- udelay(1); /* allow THRE to set */
- iir_noint1 = serial_port_in(port, UART_IIR) & UART_IIR_NO_INT;
- serial_port_out(port, UART_IER, 0);
- serial_port_out_sync(port, UART_IER, UART_IER_THRI);
- udelay(1); /* allow a working UART time to re-assert THRE */
- iir_noint2 = serial_port_in(port, UART_IIR) & UART_IIR_NO_INT;
- serial_port_out(port, UART_IER, 0);
-
- uart_port_unlock_irqrestore(port, flags);
+ scoped_guard(uart_port_lock_irqsave, port) {
+ wait_for_xmitr(up, UART_LSR_THRE);
+ serial_port_out_sync(port, UART_IER, UART_IER_THRI);
+ udelay(1); /* allow THRE to set */
+ iir_noint1 = serial_port_in(port, UART_IIR) & UART_IIR_NO_INT;
+ serial_port_out(port, UART_IER, 0);
+ serial_port_out_sync(port, UART_IER, UART_IER_THRI);
+ udelay(1); /* allow a working UART time to re-assert THRE */
+ iir_noint2 = serial_port_in(port, UART_IIR) & UART_IIR_NO_INT;
+ serial_port_out(port, UART_IER, 0);
+ }
if (port->irqflags & IRQF_SHARED)
enable_irq(port->irq);
@@ -2269,14 +2232,11 @@ static void serial8250_iir_txen_test(struct uart_port *port)
static void serial8250_initialize(struct uart_port *port)
{
- unsigned long flags;
-
- uart_port_lock_irqsave(port, &flags);
+ guard(uart_port_lock_irqsave)(port);
serial_port_out(port, UART_LCR, UART_LCR_WLEN8);
serial8250_init_mctrl(port);
serial8250_iir_txen_test(port);
- uart_port_unlock_irqrestore(port, flags);
}
int serial8250_do_startup(struct uart_port *port)
@@ -2295,7 +2255,7 @@ int serial8250_do_startup(struct uart_port *port)
if (port->iotype != up->cur_iotype)
set_io_from_upio(port);
- serial8250_rpm_get(up);
+ guard(serial8250_rpm)(up);
serial8250_startup_special(port);
@@ -2315,8 +2275,7 @@ int serial8250_do_startup(struct uart_port *port)
if (!(port->flags & UPF_BUGGY_UART) &&
(serial_port_in(port, UART_LSR) == 0xff)) {
dev_info_ratelimited(port->dev, "LSR safety check engaged!\n");
- retval = -ENODEV;
- goto out;
+ return -ENODEV;
}
serial8250_set_TRG_levels(port);
@@ -2327,7 +2286,7 @@ int serial8250_do_startup(struct uart_port *port)
retval = up->ops->setup_irq(up);
if (retval)
- goto out;
+ return retval;
serial8250_THRE_test(port);
@@ -2376,10 +2335,8 @@ int serial8250_do_startup(struct uart_port *port)
outb_p(0x80, icp);
inb_p(icp);
}
- retval = 0;
-out:
- serial8250_rpm_put(up);
- return retval;
+
+ return 0;
}
EXPORT_SYMBOL_GPL(serial8250_do_startup);
@@ -2393,7 +2350,6 @@ static int serial8250_startup(struct uart_port *port)
void serial8250_do_shutdown(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned long flags;
serial8250_rpm_get(up);
/*
@@ -2401,26 +2357,26 @@ void serial8250_do_shutdown(struct uart_port *port)
*
* Synchronize UART_IER access against the console.
*/
- uart_port_lock_irqsave(port, &flags);
- up->ier = 0;
- serial_port_out(port, UART_IER, 0);
- uart_port_unlock_irqrestore(port, flags);
+ scoped_guard(uart_port_lock_irqsave, port) {
+ up->ier = 0;
+ serial_port_out(port, UART_IER, 0);
+ }
synchronize_irq(port->irq);
if (up->dma)
serial8250_release_dma(up);
- uart_port_lock_irqsave(port, &flags);
- if (port->flags & UPF_FOURPORT) {
- /* reset interrupts on the AST Fourport board */
- inb((port->iobase & 0xfe0) | 0x1f);
- port->mctrl |= TIOCM_OUT1;
- } else
- port->mctrl &= ~TIOCM_OUT2;
+ scoped_guard(uart_port_lock_irqsave, port) {
+ if (port->flags & UPF_FOURPORT) {
+ /* reset interrupts on the AST Fourport board */
+ inb((port->iobase & 0xfe0) | 0x1f);
+ port->mctrl |= TIOCM_OUT1;
+ } else
+ port->mctrl &= ~TIOCM_OUT2;
- serial8250_set_mctrl(port, port->mctrl);
- uart_port_unlock_irqrestore(port, flags);
+ serial8250_set_mctrl(port, port->mctrl);
+ }
/*
* Disable break condition and FIFOs
@@ -2612,33 +2568,27 @@ static unsigned int serial8250_get_baud_rate(struct uart_port *port,
void serial8250_update_uartclk(struct uart_port *port, unsigned int uartclk)
{
struct tty_port *tport = &port->state->port;
- struct tty_struct *tty;
- tty = tty_port_tty_get(tport);
- if (!tty) {
- mutex_lock(&tport->mutex);
- port->uartclk = uartclk;
- mutex_unlock(&tport->mutex);
- return;
- }
+ scoped_guard(tty_port_tty, tport) {
+ struct tty_struct *tty = scoped_tty();
- down_write(&tty->termios_rwsem);
- mutex_lock(&tport->mutex);
+ guard(rwsem_write)(&tty->termios_rwsem);
+ guard(mutex)(&tport->mutex);
- if (port->uartclk == uartclk)
- goto out_unlock;
+ if (port->uartclk == uartclk)
+ return;
- port->uartclk = uartclk;
+ port->uartclk = uartclk;
- if (!tty_port_initialized(tport))
- goto out_unlock;
+ if (!tty_port_initialized(tport))
+ return;
- serial8250_do_set_termios(port, &tty->termios, NULL);
+ serial8250_do_set_termios(port, &tty->termios, NULL);
-out_unlock:
- mutex_unlock(&tport->mutex);
- up_write(&tty->termios_rwsem);
- tty_kref_put(tty);
+ return;
+ }
+ guard(mutex)(&tport->mutex);
+ port->uartclk = uartclk;
}
EXPORT_SYMBOL_GPL(serial8250_update_uartclk);
@@ -2793,7 +2743,6 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
const struct ktermios *old)
{
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned long flags;
unsigned int baud, quot, frac = 0;
u8 lcr;
@@ -2803,27 +2752,24 @@ serial8250_do_set_termios(struct uart_port *port, struct ktermios *termios,
quot = serial8250_get_divisor(port, baud, &frac);
/*
- * Ok, we're now changing the port state. Do it with
- * interrupts disabled.
+ * Ok, we're now changing the port state. Do it with interrupts disabled.
*
* Synchronize UART_IER access against the console.
*/
- serial8250_rpm_get(up);
- uart_port_lock_irqsave(port, &flags);
-
- up->lcr = lcr;
- serial8250_set_trigger_for_slow_speed(port, termios, baud);
- serial8250_set_afe(port, termios);
- uart_update_timeout(port, termios->c_cflag, baud);
- serial8250_set_errors_and_ignores(port, termios);
- serial8250_set_ier(port, termios);
- serial8250_set_efr(port, termios);
- serial8250_set_divisor(port, baud, quot, frac);
- serial8250_set_fcr(port, termios);
- serial8250_set_mctrl(port, port->mctrl);
+ scoped_guard(serial8250_rpm, up) {
+ guard(uart_port_lock_irqsave)(port);
- uart_port_unlock_irqrestore(port, flags);
- serial8250_rpm_put(up);
+ up->lcr = lcr;
+ serial8250_set_trigger_for_slow_speed(port, termios, baud);
+ serial8250_set_afe(port, termios);
+ uart_update_timeout(port, termios->c_cflag, baud);
+ serial8250_set_errors_and_ignores(port, termios);
+ serial8250_set_ier(port, termios);
+ serial8250_set_efr(port, termios);
+ serial8250_set_divisor(port, baud, quot, frac);
+ serial8250_set_fcr(port, termios);
+ serial8250_set_mctrl(port, port->mctrl);
+ }
/* Don't rewrite B0 */
if (tty_termios_baud_rate(termios))
@@ -2845,15 +2791,13 @@ void serial8250_do_set_ldisc(struct uart_port *port, struct ktermios *termios)
{
if (termios->c_line == N_PPS) {
port->flags |= UPF_HARDPPS_CD;
- uart_port_lock_irq(port);
+ guard(uart_port_lock_irq)(port);
serial8250_enable_ms(port);
- uart_port_unlock_irq(port);
} else {
port->flags &= ~UPF_HARDPPS_CD;
if (!UART_ENABLE_MS(port, termios->c_cflag)) {
- uart_port_lock_irq(port);
+ guard(uart_port_lock_irq)(port);
serial8250_disable_ms(port);
- uart_port_unlock_irq(port);
}
}
}
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/16] serial: 8250_core: use guard() in serial_unlink_irq_chain()
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (8 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 09/16] serial: 8250: " Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 11/16] serial: 8250_omap: extract omap_8250_set_termios_atomic() Jiri Slaby (SUSE)
` (5 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having all the new guards, use them in the 8250 code. This improves
readability, makes error handling easier, and marks locked portions of
code explicit.
serial_unlink_irq_chain() is done separately here because with the
guard() used, those BUG_ON()s can be switched WARN_ON()s as we can
actually handle the conditions and return (despite something went really
wrong).
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/serial/8250/8250_core.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 82c3636451e5..7d931693b311 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -178,20 +178,22 @@ static void serial_unlink_irq_chain(struct uart_8250_port *up)
{
struct irq_info *i;
- mutex_lock(&hash_mutex);
+ guard(mutex)(&hash_mutex);
hash_for_each_possible(irq_lists, i, node, up->port.irq)
- if (i->irq == up->port.irq)
- break;
+ if (i->irq == up->port.irq) {
+ if (WARN_ON(i->head == NULL))
+ return;
- BUG_ON(i == NULL);
- BUG_ON(i->head == NULL);
+ if (list_empty(i->head))
+ free_irq(up->port.irq, i);
- if (list_empty(i->head))
- free_irq(up->port.irq, i);
+ serial_do_unlink(i, up);
+
+ return;
+ }
- serial_do_unlink(i, up);
- mutex_unlock(&hash_mutex);
+ WARN_ON(1);
}
/*
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 11/16] serial: 8250_omap: extract omap_8250_set_termios_atomic()
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (9 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 10/16] serial: 8250_core: use guard() in serial_unlink_irq_chain() Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 12/16] serial: 8250_omap: use guard()s Jiri Slaby (SUSE)
` (4 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
To use guard()s easily in omap_8250_set_termios(), split it into atomic
and non-atomic part. The former can be easily guarded -- without a need
of indenting or moving code.
omap_8250_set_termios() would likely profit from a cleanup similar to
one in serial8250_do_set_termios() in commit cdc4a3e0b235 ("serial:
8250: extract serial8250_set_fcr()") and earlier.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/serial/8250/8250_omap.c | 39 ++++++++++++++++++-----------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 6707f55bdbe7..ba03955fdc6e 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -369,18 +369,12 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
serial8250_em485_stop_tx(up, true);
}
-/*
- * OMAP can use "CLK / (16 or 13) / div" for baud rate. And then we have have
- * some differences in how we want to handle flow control.
- */
-static void omap_8250_set_termios(struct uart_port *port,
- struct ktermios *termios,
- const struct ktermios *old)
+static void omap_8250_set_termios_atomic(struct uart_port *port, struct ktermios *termios,
+ const struct ktermios *old, unsigned int baud)
{
struct uart_8250_port *up = up_to_u8250p(port);
struct omap8250_priv *priv = port->private_data;
- unsigned char cval = 0;
- unsigned int baud;
+ u8 cval;
cval = UART_LCR_WLEN(tty_get_char_size(termios->c_cflag));
@@ -393,12 +387,6 @@ static void omap_8250_set_termios(struct uart_port *port,
if (termios->c_cflag & CMSPAR)
cval |= UART_LCR_SPAR;
- /*
- * Ask the core to calculate the divisor for us.
- */
- baud = uart_get_baud_rate(port, termios, old,
- port->uartclk / 16 / UART_DIV_MAX,
- port->uartclk / 13);
omap_8250_get_divisor(port, baud, priv);
/*
@@ -518,6 +506,27 @@ static void omap_8250_set_termios(struct uart_port *port,
uart_port_unlock_irq(&up->port);
pm_runtime_mark_last_busy(port->dev);
pm_runtime_put_autosuspend(port->dev);
+}
+
+/*
+ * OMAP can use "CLK / (16 or 13) / div" for baud rate. And then we have have
+ * some differences in how we want to handle flow control.
+ */
+static void omap_8250_set_termios(struct uart_port *port,
+ struct ktermios *termios,
+ const struct ktermios *old)
+{
+ struct omap8250_priv *priv = port->private_data;
+ unsigned int baud;
+
+ /*
+ * Ask the core to calculate the divisor for us.
+ */
+ baud = uart_get_baud_rate(port, termios, old,
+ port->uartclk / 16 / UART_DIV_MAX,
+ port->uartclk / 13);
+
+ omap_8250_set_termios_atomic(port, termios, old, baud);
/* calculate wakeup latency constraint */
priv->calc_latency = USEC_PER_SEC * 64 * 8 / baud;
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 12/16] serial: 8250_omap: use guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (10 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 11/16] serial: 8250_omap: extract omap_8250_set_termios_atomic() Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 13/16] serial: 8250_rsa: " Jiri Slaby (SUSE)
` (3 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having all the new guards, use them in the 8250_omap code. This improves
readability, makes error handling easier, and marks locked portions of
code explicit.
For this to work, UART_CAP_RPM has to be set to up->capabilities a bit
earlier.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/serial/8250/8250_omap.c | 108 ++++++++++------------------
1 file changed, 37 insertions(+), 71 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index ba03955fdc6e..bb23afdd63f2 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -393,8 +393,8 @@ static void omap_8250_set_termios_atomic(struct uart_port *port, struct ktermios
* Ok, we're now changing the port state. Do it with
* interrupts disabled.
*/
- pm_runtime_get_sync(port->dev);
- uart_port_lock_irq(port);
+ guard(serial8250_rpm)(up);
+ guard(uart_port_lock_irq)(port);
/*
* Update the per-port timeout.
@@ -502,10 +502,6 @@ static void omap_8250_set_termios_atomic(struct uart_port *port, struct ktermios
}
}
omap8250_restore_regs(up);
-
- uart_port_unlock_irq(&up->port);
- pm_runtime_mark_last_busy(port->dev);
- pm_runtime_put_autosuspend(port->dev);
}
/*
@@ -546,10 +542,9 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state,
struct uart_8250_port *up = up_to_u8250p(port);
u8 efr;
- pm_runtime_get_sync(port->dev);
-
+ guard(serial8250_rpm)(up);
/* Synchronize UART_IER access against the console. */
- uart_port_lock_irq(port);
+ guard(uart_port_lock_irq)(port);
serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
efr = serial_in(up, UART_EFR);
@@ -560,11 +555,6 @@ static void omap_8250_pm(struct uart_port *port, unsigned int state,
serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
serial_out(up, UART_EFR, efr);
serial_out(up, UART_LCR, 0);
-
- uart_port_unlock_irq(port);
-
- pm_runtime_mark_last_busy(port->dev);
- pm_runtime_put_autosuspend(port->dev);
}
static void omap_serial_fill_features_erratas(struct uart_8250_port *up,
@@ -736,7 +726,11 @@ static int omap_8250_startup(struct uart_port *port)
return ret;
}
- pm_runtime_get_sync(port->dev);
+#ifdef CONFIG_PM
+ up->capabilities |= UART_CAP_RPM;
+#endif
+
+ guard(serial8250_rpm)(up);
serial_out(up, UART_FCR, UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
@@ -759,14 +753,10 @@ static int omap_8250_startup(struct uart_port *port)
}
/* Synchronize UART_IER access against the console. */
- uart_port_lock_irq(port);
- up->ier = UART_IER_RLSI | UART_IER_RDI;
- serial_out(up, UART_IER, up->ier);
- uart_port_unlock_irq(port);
-
-#ifdef CONFIG_PM
- up->capabilities |= UART_CAP_RPM;
-#endif
+ scoped_guard(uart_port_lock_irq, port) {
+ up->ier = UART_IER_RLSI | UART_IER_RDI;
+ serial_out(up, UART_IER, up->ier);
+ }
/* Enable module level wake up */
priv->wer = OMAP_UART_WER_MOD_WKUP;
@@ -775,15 +765,12 @@ static int omap_8250_startup(struct uart_port *port)
serial_out(up, UART_OMAP_WER, priv->wer);
if (up->dma && !(priv->habit & UART_HAS_EFR2)) {
- uart_port_lock_irq(port);
+ guard(uart_port_lock_irq)(port);
up->dma->rx_dma(up);
- uart_port_unlock_irq(port);
}
enable_irq(port->irq);
- pm_runtime_mark_last_busy(port->dev);
- pm_runtime_put_autosuspend(port->dev);
return 0;
}
@@ -792,7 +779,7 @@ static void omap_8250_shutdown(struct uart_port *port)
struct uart_8250_port *up = up_to_u8250p(port);
struct omap8250_priv *priv = port->private_data;
- pm_runtime_get_sync(port->dev);
+ guard(serial8250_rpm)(up);
flush_work(&priv->qos_work);
if (up->dma)
@@ -803,10 +790,11 @@ static void omap_8250_shutdown(struct uart_port *port)
serial_out(up, UART_OMAP_EFR2, 0x0);
/* Synchronize UART_IER access against the console. */
- uart_port_lock_irq(port);
- up->ier = 0;
- serial_out(up, UART_IER, 0);
- uart_port_unlock_irq(port);
+ scoped_guard(uart_port_lock_irq, port) {
+ up->ier = 0;
+ serial_out(up, UART_IER, 0);
+ }
+
disable_irq_nosync(port->irq);
dev_pm_clear_wake_irq(port->dev);
@@ -819,46 +807,33 @@ static void omap_8250_shutdown(struct uart_port *port)
if (up->lcr & UART_LCR_SBC)
serial_out(up, UART_LCR, up->lcr & ~UART_LCR_SBC);
serial_out(up, UART_FCR, UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
-
- pm_runtime_mark_last_busy(port->dev);
- pm_runtime_put_autosuspend(port->dev);
}
static void omap_8250_throttle(struct uart_port *port)
{
struct omap8250_priv *priv = port->private_data;
- unsigned long flags;
- pm_runtime_get_sync(port->dev);
+ guard(serial8250_rpm)(up_to_u8250p(port));
+ guard(uart_port_lock_irqsave)(port);
- uart_port_lock_irqsave(port, &flags);
port->ops->stop_rx(port);
priv->throttled = true;
- uart_port_unlock_irqrestore(port, flags);
-
- pm_runtime_mark_last_busy(port->dev);
- pm_runtime_put_autosuspend(port->dev);
}
static void omap_8250_unthrottle(struct uart_port *port)
{
struct omap8250_priv *priv = port->private_data;
struct uart_8250_port *up = up_to_u8250p(port);
- unsigned long flags;
-
- pm_runtime_get_sync(port->dev);
+ guard(serial8250_rpm)(up);
/* Synchronize UART_IER access against the console. */
- uart_port_lock_irqsave(port, &flags);
+ guard(uart_port_lock_irqsave)(port);
+
priv->throttled = false;
if (up->dma)
up->dma->rx_dma(up);
up->ier |= UART_IER_RLSI | UART_IER_RDI;
serial_out(up, UART_IER, up->ier);
- uart_port_unlock_irqrestore(port, flags);
-
- pm_runtime_mark_last_busy(port->dev);
- pm_runtime_put_autosuspend(port->dev);
}
static int omap8250_rs485_config(struct uart_port *port,
@@ -996,30 +971,26 @@ static void __dma_rx_complete(void *param)
struct omap8250_priv *priv = p->port.private_data;
struct uart_8250_dma *dma = p->dma;
struct dma_tx_state state;
- unsigned long flags;
/* Synchronize UART_IER access against the console. */
- uart_port_lock_irqsave(&p->port, &flags);
+ guard(uart_port_lock_irqsave)(&p->port);
/*
* If the tx status is not DMA_COMPLETE, then this is a delayed
* completion callback. A previous RX timeout flush would have
* already pushed the data, so exit.
*/
- if (dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state) !=
- DMA_COMPLETE) {
- uart_port_unlock_irqrestore(&p->port, flags);
+ if (dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state) != DMA_COMPLETE)
return;
- }
+
__dma_rx_do_complete(p);
- if (!priv->throttled) {
- p->ier |= UART_IER_RLSI | UART_IER_RDI;
- serial_out(p, UART_IER, p->ier);
- if (!(priv->habit & UART_HAS_EFR2))
- omap_8250_rx_dma(p);
- }
+ if (priv->throttled)
+ return;
- uart_port_unlock_irqrestore(&p->port, flags);
+ p->ier |= UART_IER_RLSI | UART_IER_RDI;
+ serial_out(p, UART_IER, p->ier);
+ if (!(priv->habit & UART_HAS_EFR2))
+ omap_8250_rx_dma(p);
}
static void omap_8250_rx_dma_flush(struct uart_8250_port *p)
@@ -1117,14 +1088,13 @@ static void omap_8250_dma_tx_complete(void *param)
struct uart_8250_port *p = param;
struct uart_8250_dma *dma = p->dma;
struct tty_port *tport = &p->port.state->port;
- unsigned long flags;
bool en_thri = false;
struct omap8250_priv *priv = p->port.private_data;
dma_sync_single_for_cpu(dma->txchan->device->dev, dma->tx_addr,
UART_XMIT_SIZE, DMA_TO_DEVICE);
- uart_port_lock_irqsave(&p->port, &flags);
+ guard(uart_port_lock_irqsave)(&p->port);
dma->tx_running = 0;
@@ -1152,8 +1122,6 @@ static void omap_8250_dma_tx_complete(void *param)
dma->tx_err = 1;
serial8250_set_THRI(p);
}
-
- uart_port_unlock_irqrestore(&p->port, flags);
}
static int omap_8250_tx_dma(struct uart_8250_port *p)
@@ -1804,15 +1772,13 @@ static int omap8250_runtime_resume(struct device *dev)
up = serial8250_get_port(priv->line);
if (up && omap8250_lost_context(up)) {
- uart_port_lock_irq(&up->port);
+ guard(uart_port_lock_irq)(&up->port);
omap8250_restore_regs(up);
- uart_port_unlock_irq(&up->port);
}
if (up && up->dma && up->dma->rxchan && !(priv->habit & UART_HAS_EFR2)) {
- uart_port_lock_irq(&up->port);
+ guard(uart_port_lock_irq)(&up->port);
omap_8250_rx_dma(up);
- uart_port_unlock_irq(&up->port);
}
atomic_set(&priv->active, 1);
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 13/16] serial: 8250_rsa: use guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (11 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 12/16] serial: 8250_omap: use guard()s Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 14/16] tty/vt: use guard()s in con_font_set/get() and con_{set,get}_unimap() Jiri Slaby (SUSE)
` (2 subsequent siblings)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having all the new guards, use them in the 8250_rsa code. This improves
readability, makes error handling easier, and marks locked portions of
code explicit.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/serial/8250/8250_rsa.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_rsa.c b/drivers/tty/serial/8250/8250_rsa.c
index d34093cc03ad..ff52ad02f6be 100644
--- a/drivers/tty/serial/8250/8250_rsa.c
+++ b/drivers/tty/serial/8250/8250_rsa.c
@@ -140,9 +140,8 @@ void rsa_enable(struct uart_8250_port *up)
return;
if (up->port.uartclk != SERIAL_RSA_BAUD_BASE * 16) {
- uart_port_lock_irq(&up->port);
+ guard(uart_port_lock_irq)(&up->port);
__rsa_enable(up);
- uart_port_unlock_irq(&up->port);
}
if (up->port.uartclk == SERIAL_RSA_BAUD_BASE * 16)
serial_out(up, UART_RSA_FRR, 0);
@@ -165,7 +164,8 @@ void rsa_disable(struct uart_8250_port *up)
if (up->port.uartclk != SERIAL_RSA_BAUD_BASE * 16)
return;
- uart_port_lock_irq(&up->port);
+ guard(uart_port_lock_irq)(&up->port);
+
mode = serial_in(up, UART_RSA_MSR);
result = !(mode & UART_RSA_MSR_FIFO);
@@ -177,7 +177,6 @@ void rsa_disable(struct uart_8250_port *up)
if (result)
up->port.uartclk = SERIAL_RSA_BAUD_BASE_LO * 16;
- uart_port_unlock_irq(&up->port);
}
EXPORT_SYMBOL_GPL_FOR_MODULES(rsa_disable, "8250_base");
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 14/16] tty/vt: use guard()s in con_font_set/get() and con_{set,get}_unimap()
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (12 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 13/16] serial: 8250_rsa: " Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 15/16] tty/vt: use guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 16/16] s390/char/con3270: use tty_port_tty guard() Jiri Slaby (SUSE)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE)
Having all the new guards, use them in the 8250_rsa code. This improves
readability, makes error handling easier, and marks locked portions of
code explicit.
The new __free()-annotated declarations are moved to the allocation points
as is preferred:
https://lore.kernel.org/all/CAHk-=wjvh_LUpa=864joG2JJXs3+viO-kLzLidR2JLyMr4MNwA@mail.gmail.com/
Except font_data in con_font_get(). The scope in there would not match
the expected lifetime. But the declaration is moved closer.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
drivers/tty/vt/consolemap.c | 84 ++++++++++++++++------------------
drivers/tty/vt/vt.c | 89 +++++++++++++++++--------------------
2 files changed, 78 insertions(+), 95 deletions(-)
diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index bb4bb272ebec..8eb9d745a868 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -637,32 +637,28 @@ static struct uni_pagedict *con_unshare_unimap(struct vc_data *vc,
int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
{
- int err = 0, err1;
struct uni_pagedict *dict;
- struct unipair *unilist, *plist;
+ struct unipair *plist;
+ int err = 0;
if (!ct)
return 0;
- unilist = vmemdup_array_user(list, ct, sizeof(*unilist));
+ struct unipair *unilist __free(kvfree) = vmemdup_array_user(list, ct, sizeof(*unilist));
if (IS_ERR(unilist))
return PTR_ERR(unilist);
- console_lock();
+ guard(console_lock)();
/* Save original vc_unipagdir_loc in case we allocate a new one */
dict = *vc->uni_pagedict_loc;
- if (!dict) {
- err = -EINVAL;
- goto out_unlock;
- }
+ if (!dict)
+ return -EINVAL;
if (dict->refcount > 1) {
dict = con_unshare_unimap(vc, dict);
- if (IS_ERR(dict)) {
- err = PTR_ERR(dict);
- goto out_unlock;
- }
+ if (IS_ERR(dict))
+ return PTR_ERR(dict);
} else if (dict == dflt) {
dflt = NULL;
}
@@ -671,7 +667,7 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
* Insert user specified unicode pairs into new table.
*/
for (plist = unilist; ct; ct--, plist++) {
- err1 = con_insert_unipair(dict, plist->unicode, plist->fontpos);
+ int err1 = con_insert_unipair(dict, plist->unicode, plist->fontpos);
if (err1)
err = err1;
}
@@ -680,15 +676,12 @@ int con_set_unimap(struct vc_data *vc, ushort ct, struct unipair __user *list)
* Merge with fontmaps of any other virtual consoles.
*/
if (con_unify_unimap(vc, dict))
- goto out_unlock;
+ return err;
for (enum translation_map m = FIRST_MAP; m <= LAST_MAP; m++)
set_inverse_transl(vc, dict, m);
set_inverse_trans_unicode(dict);
-out_unlock:
- console_unlock();
- kvfree(unilist);
return err;
}
@@ -787,50 +780,49 @@ int con_get_unimap(struct vc_data *vc, ushort ct, ushort __user *uct,
{
ushort ect;
struct uni_pagedict *dict;
- struct unipair *unilist;
unsigned int d, r, g;
- int ret = 0;
- unilist = kvmalloc_array(ct, sizeof(*unilist), GFP_KERNEL);
+ struct unipair *unilist __free(kvfree) = kvmalloc_array(ct, sizeof(*unilist), GFP_KERNEL);
if (!unilist)
return -ENOMEM;
- console_lock();
-
- ect = 0;
- dict = *vc->uni_pagedict_loc;
- if (!dict)
- goto unlock;
-
- for (d = 0; d < UNI_DIRS; d++) {
- u16 **dir = dict->uni_pgdir[d];
- if (!dir)
- continue;
+ scoped_guard(console_lock) {
+ ect = 0;
+ dict = *vc->uni_pagedict_loc;
+ if (!dict)
+ break;
- for (r = 0; r < UNI_DIR_ROWS; r++) {
- u16 *row = dir[r];
- if (!row)
+ for (d = 0; d < UNI_DIRS; d++) {
+ u16 **dir = dict->uni_pgdir[d];
+ if (!dir)
continue;
- for (g = 0; g < UNI_ROW_GLYPHS; g++, row++) {
- if (*row >= MAX_GLYPH)
+ for (r = 0; r < UNI_DIR_ROWS; r++) {
+ u16 *row = dir[r];
+ if (!row)
continue;
- if (ect < ct) {
- unilist[ect].unicode = UNI(d, r, g);
- unilist[ect].fontpos = *row;
+
+ for (g = 0; g < UNI_ROW_GLYPHS; g++, row++) {
+ if (*row >= MAX_GLYPH)
+ continue;
+ if (ect < ct) {
+ unilist[ect].unicode = UNI(d, r, g);
+ unilist[ect].fontpos = *row;
+ }
+ ect++;
}
- ect++;
}
}
}
-unlock:
- console_unlock();
+
if (copy_to_user(list, unilist, min(ect, ct) * sizeof(*unilist)))
- ret = -EFAULT;
+ return -EFAULT;
if (put_user(ect, uct))
- ret = -EFAULT;
- kvfree(unilist);
- return ret ? ret : (ect <= ct) ? 0 : -ENOMEM;
+ return -EFAULT;
+ if (ect > ct)
+ return -ENOMEM;
+
+ return 0;
}
/*
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 62049ceb34de..100d6cb26887 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4801,57 +4801,51 @@ void reset_palette(struct vc_data *vc)
static int con_font_get(struct vc_data *vc, struct console_font_op *op)
{
struct console_font font;
- int rc = -EINVAL;
int c;
unsigned int vpitch = op->op == KD_FONT_OP_GET_TALL ? op->height : 32;
if (vpitch > max_font_height)
return -EINVAL;
+ void *font_data __free(kvfree) = NULL;
if (op->data) {
- font.data = kvzalloc(max_font_size, GFP_KERNEL);
+ font.data = font_data = kvzalloc(max_font_size, GFP_KERNEL);
if (!font.data)
return -ENOMEM;
} else
font.data = NULL;
- console_lock();
- if (vc->vc_mode != KD_TEXT)
- rc = -EINVAL;
- else if (vc->vc_sw->con_font_get)
- rc = vc->vc_sw->con_font_get(vc, &font, vpitch);
- else
- rc = -ENOSYS;
- console_unlock();
+ scoped_guard(console_lock) {
+ if (vc->vc_mode != KD_TEXT)
+ return -EINVAL;
+ if (!vc->vc_sw->con_font_get)
+ return -ENOSYS;
- if (rc)
- goto out;
+ int ret = vc->vc_sw->con_font_get(vc, &font, vpitch);
+ if (ret)
+ return ret;
+ }
c = (font.width+7)/8 * vpitch * font.charcount;
if (op->data && font.charcount > op->charcount)
- rc = -ENOSPC;
+ return -ENOSPC;
if (font.width > op->width || font.height > op->height)
- rc = -ENOSPC;
- if (rc)
- goto out;
+ return -ENOSPC;
op->height = font.height;
op->width = font.width;
op->charcount = font.charcount;
if (op->data && copy_to_user(op->data, font.data, c))
- rc = -EFAULT;
+ return -EFAULT;
-out:
- kvfree(font.data);
- return rc;
+ return 0;
}
static int con_font_set(struct vc_data *vc, const struct console_font_op *op)
{
struct console_font font;
- int rc = -EINVAL;
int size;
unsigned int vpitch = op->op == KD_FONT_OP_SET_TALL ? op->height : 32;
@@ -4870,7 +4864,7 @@ static int con_font_set(struct vc_data *vc, const struct console_font_op *op)
if (size > max_font_size)
return -ENOSPC;
- font.data = memdup_user(op->data, size);
+ void *font_data __free(kfree) = font.data = memdup_user(op->data, size);
if (IS_ERR(font.data))
return PTR_ERR(font.data);
@@ -4878,18 +4872,17 @@ static int con_font_set(struct vc_data *vc, const struct console_font_op *op)
font.width = op->width;
font.height = op->height;
- console_lock();
+ guard(console_lock)();
+
if (vc->vc_mode != KD_TEXT)
- rc = -EINVAL;
- else if (vc->vc_sw->con_font_set) {
- if (vc_is_sel(vc))
- clear_selection();
- rc = vc->vc_sw->con_font_set(vc, &font, vpitch, op->flags);
- } else
- rc = -ENOSYS;
- console_unlock();
- kfree(font.data);
- return rc;
+ return -EINVAL;
+ if (!vc->vc_sw->con_font_set)
+ return -ENOSYS;
+
+ if (vc_is_sel(vc))
+ clear_selection();
+
+ return vc->vc_sw->con_font_set(vc, &font, vpitch, op->flags);
}
static int con_font_default(struct vc_data *vc, struct console_font_op *op)
@@ -4897,8 +4890,6 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op)
struct console_font font = {.width = op->width, .height = op->height};
char name[MAX_FONT_NAME];
char *s = name;
- int rc;
-
if (!op->data)
s = NULL;
@@ -4907,23 +4898,23 @@ static int con_font_default(struct vc_data *vc, struct console_font_op *op)
else
name[MAX_FONT_NAME - 1] = 0;
- console_lock();
- if (vc->vc_mode != KD_TEXT) {
- console_unlock();
- return -EINVAL;
- }
- if (vc->vc_sw->con_font_default) {
+ scoped_guard(console_lock) {
+ if (vc->vc_mode != KD_TEXT)
+ return -EINVAL;
+ if (!vc->vc_sw->con_font_default)
+ return -ENOSYS;
+
if (vc_is_sel(vc))
clear_selection();
- rc = vc->vc_sw->con_font_default(vc, &font, s);
- } else
- rc = -ENOSYS;
- console_unlock();
- if (!rc) {
- op->width = font.width;
- op->height = font.height;
+ int ret = vc->vc_sw->con_font_default(vc, &font, s);
+ if (ret)
+ return ret;
}
- return rc;
+
+ op->width = font.width;
+ op->height = font.height;
+
+ return 0;
}
int con_font_op(struct vc_data *vc, struct console_font_op *op)
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 15/16] tty/vt: use guard()s
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (13 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 14/16] tty/vt: use guard()s in con_font_set/get() and con_{set,get}_unimap() Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 16/16] s390/char/con3270: use tty_port_tty guard() Jiri Slaby (SUSE)
15 siblings, 0 replies; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE), linux-mm
Having all the new guards, use them in the vt code. This improves
readability, makes error handling easier, and marks locked portions of
code explicit.
A local free_page_ptr __free guard is introduced for
__get_free_page/free_page (with proper casts). This could be made public
in include/. But I am not sure if there are more possible users, so
keeping completely private here.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
Cc: linux-mm@kvack.org
MM fellows,
just so you know:
Apart from here included (and easy to define) __get_free_page() guard:
DEFINE_FREE(free_page_ptr, void *, if (_T) free_page((unsigned long)_T));
it also could be extended to __get_free_pages(). But likely the only
user which would profit from that one is raid6_select_algo(). And it's
not that straighforward. Adding the free_pages guard for reference:
struct gfp_holder {
union {
unsigned long mem;
void *ptr;
};
unsigned order;
};
DEFINE_CLASS(get_free_pages, struct gfp_holder, free_pages(_T.mem, _T.order),
{ .order = order }; t.mem = __get_free_pages(gfp, order),
gfp_t gfp, unsigned order);
#define ___get_free_pages_free(var, what, gfp, order, id) \
CLASS(get_free_pages, id)(gfp, order); \
var = id.what
#define __get_free_pages_free(var, gfp, order) \
___get_free_pages_free(var, mem, gfp, order, __UNIQUE_ID(xxx))
#define __get_free_pages_ptr_free(var, gfp, order) \
___get_free_pages_free(var, ptr, gfp, order, __UNIQUE_ID(xxx))
Use like:
__get_free_pages_ptr_free(char *disk_ptr, GFP_KERNEL, RAID6_TEST_DISKS_ORDER);
if (!disk_ptr)
return -ENOMEM;
disk_ptr[0] = 10;
---
drivers/tty/vt/consolemap.c | 32 +++---
drivers/tty/vt/selection.c | 20 ++--
drivers/tty/vt/vc_screen.c | 74 ++++++--------
drivers/tty/vt/vt.c | 98 ++++++++-----------
drivers/tty/vt/vt_ioctl.c | 190 ++++++++++++++++--------------------
5 files changed, 170 insertions(+), 244 deletions(-)
diff --git a/drivers/tty/vt/consolemap.c b/drivers/tty/vt/consolemap.c
index 8eb9d745a868..7a11c3f2e875 100644
--- a/drivers/tty/vt/consolemap.c
+++ b/drivers/tty/vt/consolemap.c
@@ -361,10 +361,10 @@ int con_set_trans_old(unsigned char __user * arg)
inbuf[i] = UNI_DIRECT_BASE | ch;
}
- console_lock();
+ guard(console_lock)();
memcpy(translations[USER_MAP], inbuf, sizeof(inbuf));
update_user_maps();
- console_unlock();
+
return 0;
}
@@ -374,13 +374,11 @@ int con_get_trans_old(unsigned char __user * arg)
unsigned short *p = translations[USER_MAP];
unsigned char outbuf[E_TABSZ];
- console_lock();
- for (i = 0; i < ARRAY_SIZE(outbuf); i++)
- {
- ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
- outbuf[i] = (ch & ~0xff) ? 0 : ch;
- }
- console_unlock();
+ scoped_guard(console_lock)
+ for (i = 0; i < ARRAY_SIZE(outbuf); i++) {
+ ch = conv_uni_to_pc(vc_cons[fg_console].d, p[i]);
+ outbuf[i] = (ch & ~0xff) ? 0 : ch;
+ }
return copy_to_user(arg, outbuf, sizeof(outbuf)) ? -EFAULT : 0;
}
@@ -392,10 +390,10 @@ int con_set_trans_new(ushort __user * arg)
if (copy_from_user(inbuf, arg, sizeof(inbuf)))
return -EFAULT;
- console_lock();
+ guard(console_lock)();
memcpy(translations[USER_MAP], inbuf, sizeof(inbuf));
update_user_maps();
- console_unlock();
+
return 0;
}
@@ -403,9 +401,8 @@ int con_get_trans_new(ushort __user * arg)
{
unsigned short outbuf[E_TABSZ];
- console_lock();
- memcpy(outbuf, translations[USER_MAP], sizeof(outbuf));
- console_unlock();
+ scoped_guard(console_lock)
+ memcpy(outbuf, translations[USER_MAP], sizeof(outbuf));
return copy_to_user(arg, outbuf, sizeof(outbuf)) ? -EFAULT : 0;
}
@@ -571,11 +568,8 @@ static int con_do_clear_unimap(struct vc_data *vc)
int con_clear_unimap(struct vc_data *vc)
{
- int ret;
- console_lock();
- ret = con_do_clear_unimap(vc);
- console_unlock();
- return ret;
+ guard(console_lock)();
+ return con_do_clear_unimap(vc);
}
static struct uni_pagedict *con_unshare_unimap(struct vc_data *vc,
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index 24b0a53e5a79..07d3b93975d3 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -127,9 +127,8 @@ int sel_loadlut(u32 __user *lut)
if (copy_from_user(tmplut, lut, sizeof(inwordLut)))
return -EFAULT;
- console_lock();
+ guard(console_lock)();
memcpy(inwordLut, tmplut, sizeof(inwordLut));
- console_unlock();
return 0;
}
@@ -375,15 +374,9 @@ static int vc_selection(struct vc_data *vc, struct tiocl_selection *v,
int set_selection_kernel(struct tiocl_selection *v, struct tty_struct *tty)
{
- int ret;
-
- mutex_lock(&vc_sel.lock);
- console_lock();
- ret = vc_selection(vc_cons[fg_console].d, v, tty);
- console_unlock();
- mutex_unlock(&vc_sel.lock);
-
- return ret;
+ guard(mutex)(&vc_sel.lock);
+ guard(console_lock)();
+ return vc_selection(vc_cons[fg_console].d, v, tty);
}
EXPORT_SYMBOL_GPL(set_selection_kernel);
@@ -409,9 +402,8 @@ int paste_selection(struct tty_struct *tty)
const char *bps = bp ? bracketed_paste_start : NULL;
const char *bpe = bp ? bracketed_paste_end : NULL;
- console_lock();
- poke_blanked_console();
- console_unlock();
+ scoped_guard(console_lock)
+ poke_blanked_console();
ld = tty_ldisc_ref_wait(tty);
if (!ld)
diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 79b33d998d43..c814644ef4ee 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -53,6 +53,8 @@
#define HEADER_SIZE 4u
#define CON_BUF_SIZE (IS_ENABLED(CONFIG_BASE_SMALL) ? 256 : PAGE_SIZE)
+DEFINE_FREE(free_page_ptr, void *, if (_T) free_page((unsigned long)_T));
+
/*
* Our minor space:
*
@@ -72,7 +74,6 @@
#define use_unicode(inode) (iminor(inode) & 64)
#define use_attributes(inode) (iminor(inode) & 128)
-
struct vcs_poll_data {
struct notifier_block notifier;
unsigned int cons_num;
@@ -231,15 +232,13 @@ static loff_t vcs_lseek(struct file *file, loff_t offset, int orig)
struct vc_data *vc;
int size;
- console_lock();
- vc = vcs_vc(inode, NULL);
- if (!vc) {
- console_unlock();
- return -ENXIO;
- }
+ scoped_guard(console_lock) {
+ vc = vcs_vc(inode, NULL);
+ if (!vc)
+ return -ENXIO;
- size = vcs_size(vc, use_attributes(inode), use_unicode(inode));
- console_unlock();
+ size = vcs_size(vc, use_attributes(inode), use_unicode(inode));
+ }
if (size < 0)
return size;
return fixed_size_llseek(file, offset, orig, size);
@@ -369,11 +368,10 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
struct vcs_poll_data *poll;
unsigned int read;
ssize_t ret;
- char *con_buf;
loff_t pos;
bool viewed, attr, uni_mode;
- con_buf = (char *) __get_free_page(GFP_KERNEL);
+ char *con_buf __free(free_page_ptr) = (char *)__get_free_page(GFP_KERNEL);
if (!con_buf)
return -ENOMEM;
@@ -382,17 +380,16 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
/* Select the proper current console and verify
* sanity of the situation under the console lock.
*/
- console_lock();
+ guard(console_lock)();
uni_mode = use_unicode(inode);
attr = use_attributes(inode);
- ret = -EINVAL;
if (pos < 0)
- goto unlock_out;
+ return -EINVAL;
/* we enforce 32-bit alignment for pos and count in unicode mode */
if (uni_mode && (pos | count) & 3)
- goto unlock_out;
+ return -EINVAL;
poll = file->private_data;
if (count && poll)
@@ -468,10 +465,8 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
}
*ppos += read;
if (read)
- ret = read;
-unlock_out:
- console_unlock();
- free_page((unsigned long) con_buf);
+ return read;
+
return ret;
}
@@ -591,7 +586,6 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
{
struct inode *inode = file_inode(file);
struct vc_data *vc;
- char *con_buf;
u16 *org0, *org;
unsigned int written;
int size;
@@ -602,7 +596,7 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (use_unicode(inode))
return -EOPNOTSUPP;
- con_buf = (char *) __get_free_page(GFP_KERNEL);
+ char *con_buf __free(free_page_ptr) = (char *)__get_free_page(GFP_KERNEL);
if (!con_buf)
return -ENOMEM;
@@ -611,22 +605,18 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
/* Select the proper current console and verify
* sanity of the situation under the console lock.
*/
- console_lock();
+ guard(console_lock)();
attr = use_attributes(inode);
- ret = -ENXIO;
vc = vcs_vc(inode, &viewed);
if (!vc)
- goto unlock_out;
+ return -ENXIO;
size = vcs_size(vc, attr, false);
- if (size < 0) {
- ret = size;
- goto unlock_out;
- }
- ret = -EINVAL;
+ if (size < 0)
+ return size;
if (pos < 0 || pos > size)
- goto unlock_out;
+ return -EINVAL;
if (count > size - pos)
count = size - pos;
written = 0;
@@ -651,8 +641,7 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
*/
if (written)
break;
- ret = -EFAULT;
- goto unlock_out;
+ return -EFAULT;
}
}
@@ -664,15 +653,13 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (!vc) {
if (written)
break;
- ret = -ENXIO;
- goto unlock_out;
+ return -ENXIO;
}
size = vcs_size(vc, attr, false);
if (size < 0) {
if (written)
break;
- ret = size;
- goto unlock_out;
+ return size;
}
if (pos >= size)
break;
@@ -702,9 +689,6 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (written)
vcs_scr_updated(vc);
-unlock_out:
- console_unlock();
- free_page((unsigned long) con_buf);
return ret;
}
@@ -754,17 +738,17 @@ vcs_open(struct inode *inode, struct file *filp)
unsigned int currcons = console(inode);
bool attr = use_attributes(inode);
bool uni_mode = use_unicode(inode);
- int ret = 0;
/* we currently don't support attributes in unicode mode */
if (attr && uni_mode)
return -EOPNOTSUPP;
- console_lock();
- if(currcons && !vc_cons_allocated(currcons-1))
- ret = -ENXIO;
- console_unlock();
- return ret;
+ guard(console_lock)();
+
+ if (currcons && !vc_cons_allocated(currcons - 1))
+ return -ENXIO;
+
+ return 0;
}
static int vcs_release(struct inode *inode, struct file *file)
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 100d6cb26887..869261141535 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1317,12 +1317,9 @@ EXPORT_SYMBOL(__vc_resize);
static int vt_resize(struct tty_struct *tty, struct winsize *ws)
{
struct vc_data *vc = tty->driver_data;
- int ret;
- console_lock();
- ret = vc_do_resize(tty, vc, ws->ws_col, ws->ws_row, false);
- console_unlock();
- return ret;
+ guard(console_lock)();
+ return vc_do_resize(tty, vc, ws->ws_col, ws->ws_row, false);
}
struct vc_data *vc_deallocate(unsigned int currcons)
@@ -3135,12 +3132,11 @@ static int do_con_write(struct tty_struct *tty, const u8 *buf, int count)
if (in_interrupt())
return count;
- console_lock();
+ guard(console_lock)();
currcons = vc->vc_num;
if (!vc_cons_allocated(currcons)) {
/* could this happen? */
pr_warn_once("con_write: tty %d not allocated\n", currcons+1);
- console_unlock();
return 0;
}
@@ -3184,7 +3180,7 @@ static int do_con_write(struct tty_struct *tty, const u8 *buf, int count)
con_flush(vc, &draw);
console_conditional_schedule();
notify_update(vc);
- console_unlock();
+
return n;
}
@@ -3199,7 +3195,7 @@ static int do_con_write(struct tty_struct *tty, const u8 *buf, int count)
*/
static void console_callback(struct work_struct *ignored)
{
- console_lock();
+ guard(console_lock)();
if (want_console >= 0) {
if (want_console != fg_console &&
@@ -3228,8 +3224,6 @@ static void console_callback(struct work_struct *ignored)
blank_timer_expired = 0;
}
notify_update(vc_cons[fg_console].d);
-
- console_unlock();
}
int set_console(int nr)
@@ -3433,9 +3427,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
return -EPERM;
return paste_selection(tty);
case TIOCL_UNBLANKSCREEN:
- console_lock();
- unblank_screen();
- console_unlock();
+ scoped_guard(console_lock)
+ unblank_screen();
break;
case TIOCL_SELLOADLUT:
if (!capable(CAP_SYS_ADMIN))
@@ -3451,9 +3444,8 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
data = vt_get_shift_state();
return put_user(data, p);
case TIOCL_GETMOUSEREPORTING:
- console_lock(); /* May be overkill */
- data = mouse_reporting();
- console_unlock();
+ scoped_guard(console_lock) /* May be overkill */
+ data = mouse_reporting();
return put_user(data, p);
case TIOCL_SETVESABLANK:
return set_vesa_blanking(param);
@@ -3484,15 +3476,14 @@ int tioclinux(struct tty_struct *tty, unsigned long arg)
* Needs the console lock here. Note that lots of other calls
* need fixing before the lock is actually useful!
*/
- console_lock();
- scrollfront(vc_cons[fg_console].d, lines);
- console_unlock();
+ scoped_guard(console_lock)
+ scrollfront(vc_cons[fg_console].d, lines);
break;
case TIOCL_BLANKSCREEN: /* until explicitly unblanked, not only poked */
- console_lock();
- ignore_poke = 1;
- do_blank_screen(0);
- console_unlock();
+ scoped_guard(console_lock) {
+ ignore_poke = 1;
+ do_blank_screen(0);
+ }
break;
case TIOCL_BLANKEDSCREEN:
return console_blanked;
@@ -3582,9 +3573,8 @@ static void con_flush_chars(struct tty_struct *tty)
if (in_interrupt()) /* from flush_to_ldisc */
return;
- console_lock();
+ guard(console_lock)();
set_cursor(vc);
- console_unlock();
}
/*
@@ -3596,22 +3586,20 @@ static int con_install(struct tty_driver *driver, struct tty_struct *tty)
struct vc_data *vc;
int ret;
- console_lock();
+ guard(console_lock)();
ret = vc_allocate(currcons);
if (ret)
- goto unlock;
+ return ret;
vc = vc_cons[currcons].d;
/* Still being freed */
- if (vc->port.tty) {
- ret = -ERESTARTSYS;
- goto unlock;
- }
+ if (vc->port.tty)
+ return -ERESTARTSYS;
ret = tty_port_install(&vc->port, driver, tty);
if (ret)
- goto unlock;
+ return ret;
tty->driver_data = vc;
vc->port.tty = tty;
@@ -3625,9 +3613,8 @@ static int con_install(struct tty_driver *driver, struct tty_struct *tty)
tty->termios.c_iflag |= IUTF8;
else
tty->termios.c_iflag &= ~IUTF8;
-unlock:
- console_unlock();
- return ret;
+
+ return 0;
}
static int con_open(struct tty_struct *tty, struct file *filp)
@@ -3646,9 +3633,9 @@ static void con_shutdown(struct tty_struct *tty)
{
struct vc_data *vc = tty->driver_data;
BUG_ON(vc == NULL);
- console_lock();
+
+ guard(console_lock)();
vc->port.tty = NULL;
- console_unlock();
}
static void con_cleanup(struct tty_struct *tty)
@@ -4137,15 +4124,13 @@ static ssize_t store_bind(struct device *dev, struct device_attribute *attr,
struct con_driver *con = dev_get_drvdata(dev);
int bind = simple_strtoul(buf, NULL, 0);
- console_lock();
+ guard(console_lock)();
if (bind)
vt_bind(con);
else
vt_unbind(con);
- console_unlock();
-
return count;
}
@@ -4155,9 +4140,8 @@ static ssize_t show_bind(struct device *dev, struct device_attribute *attr,
struct con_driver *con = dev_get_drvdata(dev);
int bind;
- console_lock();
- bind = con_is_bound(con->con);
- console_unlock();
+ scoped_guard(console_lock)
+ bind = con_is_bound(con->con);
return sysfs_emit(buf, "%i\n", bind);
}
@@ -4429,7 +4413,7 @@ static void con_driver_unregister_callback(struct work_struct *ignored)
{
int i;
- console_lock();
+ guard(console_lock)();
for (i = 0; i < MAX_NR_CON_DRIVER; i++) {
struct con_driver *con_driver = ®istered_con_driver[i];
@@ -4454,8 +4438,6 @@ static void con_driver_unregister_callback(struct work_struct *ignored)
con_driver->first = 0;
con_driver->last = 0;
}
-
- console_unlock();
}
/*
@@ -4491,9 +4473,8 @@ EXPORT_SYMBOL_GPL(do_take_over_console);
*/
void give_up_console(const struct consw *csw)
{
- console_lock();
+ guard(console_lock)();
do_unregister_con_driver(csw);
- console_unlock();
}
EXPORT_SYMBOL(give_up_console);
@@ -4541,9 +4522,8 @@ static int set_vesa_blanking(u8 __user *mode_user)
if (get_user(mode, mode_user))
return -EFAULT;
- console_lock();
+ guard(console_lock)();
vesa_blank_mode = (mode <= VESA_BLANK_MAX) ? mode : VESA_NO_BLANKING;
- console_unlock();
return 0;
}
@@ -4729,7 +4709,7 @@ int con_set_cmap(unsigned char __user *arg)
if (copy_from_user(colormap, arg, sizeof(colormap)))
return -EFAULT;
- console_lock();
+ guard(console_lock)();
for (i = k = 0; i < 16; i++) {
default_red[i] = colormap[k++];
default_grn[i] = colormap[k++];
@@ -4745,7 +4725,6 @@ int con_set_cmap(unsigned char __user *arg)
}
set_palette(vc_cons[i].d);
}
- console_unlock();
return 0;
}
@@ -4755,13 +4734,12 @@ int con_get_cmap(unsigned char __user *arg)
int i, k;
unsigned char colormap[3*16];
- console_lock();
- for (i = k = 0; i < 16; i++) {
- colormap[k++] = default_red[i];
- colormap[k++] = default_grn[i];
- colormap[k++] = default_blu[i];
- }
- console_unlock();
+ scoped_guard(console_lock)
+ for (i = k = 0; i < 16; i++) {
+ colormap[k++] = default_red[i];
+ colormap[k++] = default_grn[i];
+ colormap[k++] = default_blu[i];
+ }
if (copy_to_user(arg, colormap, sizeof(colormap)))
return -EFAULT;
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 61342e06970a..c9f11c4bd9fe 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -373,15 +373,13 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
break;
}
- case KDSETMODE:
+ case KDSETMODE: {
if (!perm)
return -EPERM;
- console_lock();
- ret = vt_kdsetmode(vc, arg);
- console_unlock();
- return ret;
-
+ guard(console_lock)();
+ return vt_kdsetmode(vc, arg);
+ }
case KDGETMODE:
return put_user(vc->vc_mode, (int __user *)arg);
@@ -601,23 +599,21 @@ static int vt_setactivate(struct vt_setactivate __user *sa)
vsa.console--;
vsa.console = array_index_nospec(vsa.console, MAX_NR_CONSOLES);
- console_lock();
- ret = vc_allocate(vsa.console);
- if (ret) {
- console_unlock();
- return ret;
- }
+ scoped_guard(console_lock) {
+ ret = vc_allocate(vsa.console);
+ if (ret)
+ return ret;
- /*
- * This is safe providing we don't drop the console sem between
- * vc_allocate and finishing referencing nvc.
- */
- nvc = vc_cons[vsa.console].d;
- nvc->vt_mode = vsa.mode;
- nvc->vt_mode.frsig = 0;
- put_pid(nvc->vt_pid);
- nvc->vt_pid = get_pid(task_pid(current));
- console_unlock();
+ /*
+ * This is safe providing we don't drop the console sem between
+ * vc_allocate and finishing referencing nvc.
+ */
+ nvc = vc_cons[vsa.console].d;
+ nvc->vt_mode = vsa.mode;
+ nvc->vt_mode.frsig = 0;
+ put_pid(nvc->vt_pid);
+ nvc->vt_pid = get_pid(task_pid(current));
+ }
/* Commence switch and lock */
/* Review set_console locks */
@@ -630,19 +626,18 @@ static int vt_setactivate(struct vt_setactivate __user *sa)
static int vt_disallocate(unsigned int vc_num)
{
struct vc_data *vc = NULL;
- int ret = 0;
- console_lock();
- if (vt_busy(vc_num))
- ret = -EBUSY;
- else if (vc_num)
- vc = vc_deallocate(vc_num);
- console_unlock();
+ scoped_guard(console_lock) {
+ if (vt_busy(vc_num))
+ return -EBUSY;
+ if (vc_num)
+ vc = vc_deallocate(vc_num);
+ }
if (vc && vc_num >= MIN_NR_CONSOLES)
tty_port_put(&vc->port);
- return ret;
+ return 0;
}
/* deallocate all unused consoles, but leave 0 */
@@ -651,13 +646,12 @@ static void vt_disallocate_all(void)
struct vc_data *vc[MAX_NR_CONSOLES];
int i;
- console_lock();
- for (i = 1; i < MAX_NR_CONSOLES; i++)
- if (!vt_busy(i))
- vc[i] = vc_deallocate(i);
- else
- vc[i] = NULL;
- console_unlock();
+ scoped_guard(console_lock)
+ for (i = 1; i < MAX_NR_CONSOLES; i++)
+ if (!vt_busy(i))
+ vc[i] = vc_deallocate(i);
+ else
+ vc[i] = NULL;
for (i = 1; i < MAX_NR_CONSOLES; i++) {
if (vc[i] && i >= MIN_NR_CONSOLES)
@@ -703,7 +697,7 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (!vc_cons[i].d)
continue;
- console_lock();
+ guard(console_lock)();
vcp = vc_cons[i].d;
if (vcp) {
int ret;
@@ -718,11 +712,9 @@ static int vt_resizex(struct vc_data *vc, struct vt_consize __user *cs)
if (ret) {
vcp->vc_scan_lines = save_scan_lines;
vcp->vc_cell_height = save_cell_height;
- console_unlock();
return ret;
}
}
- console_unlock();
}
return 0;
@@ -770,7 +762,7 @@ int vt_ioctl(struct tty_struct *tty,
if (tmp.mode != VT_AUTO && tmp.mode != VT_PROCESS)
return -EINVAL;
- console_lock();
+ guard(console_lock)();
vc->vt_mode = tmp;
/* the frsig is ignored, so we set it to 0 */
vc->vt_mode.frsig = 0;
@@ -778,7 +770,6 @@ int vt_ioctl(struct tty_struct *tty,
vc->vt_pid = get_pid(task_pid(current));
/* no switch is required -- saw@shade.msu.ru */
vc->vt_newvt = -1;
- console_unlock();
break;
}
@@ -787,9 +778,8 @@ int vt_ioctl(struct tty_struct *tty,
struct vt_mode tmp;
int rc;
- console_lock();
- memcpy(&tmp, &vc->vt_mode, sizeof(struct vt_mode));
- console_unlock();
+ scoped_guard(console_lock)
+ memcpy(&tmp, &vc->vt_mode, sizeof(struct vt_mode));
rc = copy_to_user(up, &tmp, sizeof(struct vt_mode));
if (rc)
@@ -811,12 +801,10 @@ int vt_ioctl(struct tty_struct *tty,
return -EFAULT;
state = 1; /* /dev/tty0 is always open */
- console_lock(); /* required by vt_in_use() */
- for (i = 0, mask = 2; i < MAX_NR_CONSOLES && mask;
- ++i, mask <<= 1)
- if (vt_in_use(i))
- state |= mask;
- console_unlock();
+ scoped_guard(console_lock) /* required by vt_in_use() */
+ for (i = 0, mask = 2; i < MAX_NR_CONSOLES && mask; ++i, mask <<= 1)
+ if (vt_in_use(i))
+ state |= mask;
return put_user(state, &vtstat->v_state);
}
@@ -824,11 +812,10 @@ int vt_ioctl(struct tty_struct *tty,
* Returns the first available (non-opened) console.
*/
case VT_OPENQRY:
- console_lock(); /* required by vt_in_use() */
- for (i = 0; i < MAX_NR_CONSOLES; ++i)
- if (!vt_in_use(i))
- break;
- console_unlock();
+ scoped_guard(console_lock) /* required by vt_in_use() */
+ for (i = 0; i < MAX_NR_CONSOLES; ++i)
+ if (!vt_in_use(i))
+ break;
i = i < MAX_NR_CONSOLES ? (i+1) : -1;
return put_user(i, (int __user *)arg);
@@ -845,11 +832,11 @@ int vt_ioctl(struct tty_struct *tty,
arg--;
arg = array_index_nospec(arg, MAX_NR_CONSOLES);
- console_lock();
- ret = vc_allocate(arg);
- console_unlock();
- if (ret)
- return ret;
+ scoped_guard(console_lock) {
+ ret = vc_allocate(arg);
+ if (ret)
+ return ret;
+ }
set_console(arg);
break;
@@ -880,15 +867,13 @@ int vt_ioctl(struct tty_struct *tty,
* 2: completed switch-to OK
*/
case VT_RELDISP:
+ {
if (!perm)
return -EPERM;
- console_lock();
- ret = vt_reldisp(vc, arg);
- console_unlock();
-
- return ret;
-
+ guard(console_lock)();
+ return vt_reldisp(vc, arg);
+ }
/*
* Disallocate memory associated to VT (but leave VT1)
@@ -917,7 +902,7 @@ int vt_ioctl(struct tty_struct *tty,
get_user(cc, &vtsizes->v_cols))
return -EFAULT;
- console_lock();
+ guard(console_lock)();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
vc = vc_cons[i].d;
@@ -926,7 +911,6 @@ int vt_ioctl(struct tty_struct *tty,
__vc_resize(vc_cons[i].d, cc, ll, true);
}
}
- console_unlock();
break;
}
@@ -996,20 +980,17 @@ void vc_SAK(struct work_struct *work)
struct vc_data *vc;
struct tty_struct *tty;
- console_lock();
+ guard(console_lock)();
vc = vc_con->d;
- if (vc) {
- /* FIXME: review tty ref counting */
- tty = vc->port.tty;
- /*
- * SAK should also work in all raw modes and reset
- * them properly.
- */
- if (tty)
- __do_SAK(tty);
- reset_vc(vc);
- }
- console_unlock();
+ if (!vc)
+ return;
+
+ /* FIXME: review tty ref counting */
+ tty = vc->port.tty;
+ /* SAK should also work in all raw modes and reset them properly. */
+ if (tty)
+ __do_SAK(tty);
+ reset_vc(vc);
}
#ifdef CONFIG_COMPAT
@@ -1287,31 +1268,29 @@ int vt_move_to_console(unsigned int vt, int alloc)
{
int prev;
- console_lock();
- /* Graphics mode - up to X */
- if (disable_vt_switch) {
- console_unlock();
- return 0;
- }
- prev = fg_console;
+ scoped_guard(console_lock) {
+ /* Graphics mode - up to X */
+ if (disable_vt_switch)
+ return 0;
- if (alloc && vc_allocate(vt)) {
- /* we can't have a free VC for now. Too bad,
- * we don't want to mess the screen for now. */
- console_unlock();
- return -ENOSPC;
- }
+ prev = fg_console;
- if (set_console(vt)) {
- /*
- * We're unable to switch to the SUSPEND_CONSOLE.
- * Let the calling function know so it can decide
- * what to do.
- */
- console_unlock();
- return -EIO;
+ if (alloc && vc_allocate(vt)) {
+ /*
+ * We can't have a free VC for now. Too bad, we don't want to mess the
+ * screen for now.
+ */
+ return -ENOSPC;
+ }
+
+ if (set_console(vt)) {
+ /*
+ * We're unable to switch to the SUSPEND_CONSOLE. Let the calling function
+ * know so it can decide what to do.
+ */
+ return -EIO;
+ }
}
- console_unlock();
if (vt_waitactive(vt + 1)) {
pr_debug("Suspend: Can't switch VCs.");
return -EINTR;
@@ -1328,8 +1307,7 @@ int vt_move_to_console(unsigned int vt, int alloc)
*/
void pm_set_vt_switch(int do_switch)
{
- console_lock();
+ guard(console_lock)();
disable_vt_switch = !do_switch;
- console_unlock();
}
EXPORT_SYMBOL(pm_set_vt_switch);
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 16/16] s390/char/con3270: use tty_port_tty guard()
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
` (14 preceding siblings ...)
2025-08-14 7:24 ` [PATCH 15/16] tty/vt: use guard()s Jiri Slaby (SUSE)
@ 2025-08-14 7:24 ` Jiri Slaby (SUSE)
2025-08-14 8:19 ` Heiko Carstens
15 siblings, 1 reply; 18+ messages in thread
From: Jiri Slaby (SUSE) @ 2025-08-14 7:24 UTC (permalink / raw)
To: gregkh
Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE), Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, linux-s390
Having the new tty_port_tty guard, use it in tty3270_resize(). This
makes the code easier to read. The winsize is now defined in the
scope and initialized immediately, so that it's obvious.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
---
Cc: linux-s390@vger.kernel.org
---
drivers/s390/char/con3270.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/s390/char/con3270.c b/drivers/s390/char/con3270.c
index b78b86e8f281..a367f95c7c53 100644
--- a/drivers/s390/char/con3270.c
+++ b/drivers/s390/char/con3270.c
@@ -970,8 +970,6 @@ static void tty3270_resize(struct raw3270_view *view,
char **old_rcl_lines, **new_rcl_lines;
char *old_prompt, *new_prompt;
char *old_input, *new_input;
- struct tty_struct *tty;
- struct winsize ws;
size_t prompt_sz;
int new_allocated, old_allocated = tp->allocated_lines;
@@ -1023,14 +1021,14 @@ static void tty3270_resize(struct raw3270_view *view,
kfree(old_prompt);
tty3270_free_recall(old_rcl_lines);
tty3270_set_timer(tp, 1);
- /* Informat tty layer about new size */
- tty = tty_port_tty_get(&tp->port);
- if (!tty)
- return;
- ws.ws_row = tty3270_tty_rows(tp);
- ws.ws_col = tp->view.cols;
- tty_do_resize(tty, &ws);
- tty_kref_put(tty);
+ /* Inform the tty layer about new size */
+ scoped_guard(tty_port_tty, &tp->port) {
+ struct winsize ws = {
+ .ws_row = tty3270_tty_rows(tp),
+ .ws_col = tp->view.cols,
+ };
+ tty_do_resize(scoped_tty(), &ws);
+ }
return;
out_screen:
tty3270_free_screen(screen, new_rows);
--
2.50.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 16/16] s390/char/con3270: use tty_port_tty guard()
2025-08-14 7:24 ` [PATCH 16/16] s390/char/con3270: use tty_port_tty guard() Jiri Slaby (SUSE)
@ 2025-08-14 8:19 ` Heiko Carstens
0 siblings, 0 replies; 18+ messages in thread
From: Heiko Carstens @ 2025-08-14 8:19 UTC (permalink / raw)
To: Jiri Slaby (SUSE)
Cc: gregkh, linux-serial, linux-kernel, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
linux-s390
On Thu, Aug 14, 2025 at 09:24:56AM +0200, Jiri Slaby (SUSE) wrote:
> Having the new tty_port_tty guard, use it in tty3270_resize(). This
> makes the code easier to read. The winsize is now defined in the
> scope and initialized immediately, so that it's obvious.
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Sven Schnelle <svens@linux.ibm.com>
> ---
> Cc: linux-s390@vger.kernel.org
> ---
> drivers/s390/char/con3270.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
...
> - tty = tty_port_tty_get(&tp->port);
> - if (!tty)
> - return;
> - ws.ws_row = tty3270_tty_rows(tp);
> - ws.ws_col = tp->view.cols;
> - tty_do_resize(tty, &ws);
> - tty_kref_put(tty);
> + /* Inform the tty layer about new size */
> + scoped_guard(tty_port_tty, &tp->port) {
> + struct winsize ws = {
> + .ws_row = tty3270_tty_rows(tp),
> + .ws_col = tp->view.cols,
> + };
> + tty_do_resize(scoped_tty(), &ws);
> + }
This generates worse code compared to before, since an extra not needed
"if (IS_ERR(...))" check is added implicitly. For this particular code
it doesn't matter.
Just wanted to mention it, since this is not stated anywhere.
In any case:
Acked-by: Heiko Carstens <hca@linux.ibm.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-08-14 8:20 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-14 7:24 [PATCH 00/16] tty: use lock, rpm, and free guards Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 01/16] console: introduce console_lock guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 02/16] tty: introduce tty_port_tty guard() Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 03/16] serial: introduce uart_port_lock() guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 04/16] serial: 8250: introduce RPM guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 05/16] tty: tty_port: use guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 06/16] mxser: use tty_port_tty guard() in mxser_port_isr() Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 07/16] mxser: use guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 08/16] serial: serial_core: " Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 09/16] serial: 8250: " Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 10/16] serial: 8250_core: use guard() in serial_unlink_irq_chain() Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 11/16] serial: 8250_omap: extract omap_8250_set_termios_atomic() Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 12/16] serial: 8250_omap: use guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 13/16] serial: 8250_rsa: " Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 14/16] tty/vt: use guard()s in con_font_set/get() and con_{set,get}_unimap() Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 15/16] tty/vt: use guard()s Jiri Slaby (SUSE)
2025-08-14 7:24 ` [PATCH 16/16] s390/char/con3270: use tty_port_tty guard() Jiri Slaby (SUSE)
2025-08-14 8:19 ` Heiko Carstens
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.