All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 = &registered_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.