[PATCH 03/24] serial: 8250: don't wait for the em485 timers locked

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

From: Josef Bacik <josef@toxicpanda.com>

serial8250_em485_destroy() is called with the port lock held and
interrupts off, from ->rs485_config() and from
serial8250_unregister_port().  It calls hrtimer_cancel(), which waits
for a handler which is running, and both handlers start by taking the
port lock:

  CPU0                                  CPU1
  ioctl(TIOCSRS485), rs485 off
  uart_set_rs485_config()
    uart_port_lock_irqsave()            serial8250_em485_handle_stop_tx()
    serial8250_em485_config()             uart_port_lock_irqsave()
      serial8250_em485_destroy()            spins
        hrtimer_cancel()
          spins

All it takes is to turn RS485 off right after the end of a transmission,
when the timer for delay_rts_after_send expires.

The handlers do nothing unless em485->active_timer points to their
timer, so it is enough to clear that pointer.  What cannot be done with
the lock held is to free the structure: hrtimer_try_to_cancel() fails
for a timer whose handler is running, even if it has been started again
in the meantime.  So keep the structure in the port when RS485 is turned
off, use it again when RS485 is turned on, and free it when the port is
unregistered, where the timers can be waited for without the lock.

A port on which RS485 emulation has been used once keeps the structure,
about 150 bytes, until it is unregistered.

Fixes: 6e0a5de2136b ("serial: 8250: Use hrtimers for rs485 delays")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Tate Whiteberg <whiteberg@arista.com>
Cc: Christian Marangi <ansuelsmth@gmail.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: Fushuai Wang <wangfushuai@baidu.com>
Cc: Rafael Gago Castano <rgc@hms.se>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250.h      |  1 +
 drivers/tty/serial/8250/8250_core.c |  5 +---
 drivers/tty/serial/8250/8250_port.c | 60 ++++++++++++++++++++++++++++++-------
 include/linux/serial_8250.h         |  1 +
 4 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 21ed9c41b751..82a3b8359643 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -235,6 +235,7 @@ int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
 void serial8250_em485_start_tx(struct uart_8250_port *p, bool toggle_ier);
 void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier);
 void serial8250_em485_destroy(struct uart_8250_port *p);
+void serial8250_em485_free(struct uart_8250_port *p);
 extern struct serial_rs485 serial8250_em485_supported;
 
 /* MCR <-> TIOCM conversion */
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 42319843cb5b..0951d3e42e1f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -947,10 +947,7 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
 {
 	guard(mutex)(&serial_mutex);
 
-	if (uart->em485) {
-		guard(uart_port_lock_irqsave)(&uart->port);
-		serial8250_em485_destroy(uart);
-	}
+	serial8250_em485_free(uart);
 
 	uart_remove_one_port(&serial8250_reg, &uart->port);
 	if (serial8250_isa_devs) {
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 36b2b0a2c05c..3d47f535d6f1 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -574,15 +574,22 @@ static int serial8250_em485_init(struct uart_8250_port *p)
 	if (p->em485)
 		goto deassert_rts;
 
-	p->em485 = kmalloc_obj(struct uart_8250_em485, GFP_ATOMIC);
-	if (!p->em485)
-		return -ENOMEM;
+	if (p->em485_idle) {
+		p->em485 = p->em485_idle;
+		p->em485_idle = NULL;
+	} else {
+		p->em485 = kmalloc_obj(struct uart_8250_em485, GFP_ATOMIC);
+		if (!p->em485)
+			return -ENOMEM;
 
-	hrtimer_setup(&p->em485->stop_tx_timer, &serial8250_em485_handle_stop_tx, CLOCK_MONOTONIC,
-		      HRTIMER_MODE_REL);
-	hrtimer_setup(&p->em485->start_tx_timer, &serial8250_em485_handle_start_tx, CLOCK_MONOTONIC,
-		      HRTIMER_MODE_REL);
-	p->em485->port = p;
+		hrtimer_setup(&p->em485->stop_tx_timer,
+			      &serial8250_em485_handle_stop_tx, CLOCK_MONOTONIC,
+			      HRTIMER_MODE_REL);
+		hrtimer_setup(&p->em485->start_tx_timer,
+			      &serial8250_em485_handle_start_tx, CLOCK_MONOTONIC,
+			      HRTIMER_MODE_REL);
+		p->em485->port = p;
+	}
 	p->em485->active_timer = NULL;
 	p->em485->tx_stopped = true;
 
@@ -604,6 +611,11 @@ static int serial8250_em485_init(struct uart_8250_port *p)
  *	The function is supposed to be called from .rs485_config callback
  *	or from any other callback protected with p->port.lock spinlock.
  *
+ *	The handlers of the timers take that lock, so they cannot be waited for
+ *	here. One which is running does nothing, as no timer is active any more.
+ *	The memory is kept for the next serial8250_em485_init() and freed by
+ *	serial8250_em485_free().
+ *
  *	See also serial8250_em485_init()
  */
 void serial8250_em485_destroy(struct uart_8250_port *p)
@@ -611,14 +623,40 @@ void serial8250_em485_destroy(struct uart_8250_port *p)
 	if (!p->em485)
 		return;
 
-	hrtimer_cancel(&p->em485->start_tx_timer);
-	hrtimer_cancel(&p->em485->stop_tx_timer);
+	hrtimer_try_to_cancel(&p->em485->start_tx_timer);
+	hrtimer_try_to_cancel(&p->em485->stop_tx_timer);
+	p->em485->active_timer = NULL;
 
-	kfree(p->em485);
+	p->em485_idle = p->em485;
 	p->em485 = NULL;
 }
 EXPORT_SYMBOL_GPL(serial8250_em485_destroy);
 
+/**
+ *	serial8250_em485_free() - release the rs485 emulation of a port
+ *	@p:	uart_8250_port port instance
+ *
+ *	Must be called without p->port.lock, as it waits for the timers.
+ */
+void serial8250_em485_free(struct uart_8250_port *p)
+{
+	struct uart_8250_em485 *em485;
+
+	scoped_guard(uart_port_lock_irqsave, &p->port) {
+		serial8250_em485_destroy(p);
+		em485 = p->em485_idle;
+		p->em485_idle = NULL;
+	}
+
+	if (!em485)
+		return;
+
+	hrtimer_cancel(&em485->start_tx_timer);
+	hrtimer_cancel(&em485->stop_tx_timer);
+	kfree(em485);
+}
+EXPORT_SYMBOL_GPL(serial8250_em485_free);
+
 struct serial_rs485 serial8250_em485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
 		 SER_RS485_TERMINATE_BUS | SER_RS485_RX_DURING_TX,
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index d91c0477be9d..0556bb722db4 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -173,6 +173,7 @@ struct uart_8250_port {
 	void			(*dl_write)(struct uart_8250_port *up, u32 value);
 
 	struct uart_8250_em485 *em485;
+	struct uart_8250_em485 *em485_idle;	/* kept while rs485 is off */
 	void			(*rs485_start_tx)(struct uart_8250_port *up, bool toggle_ier);
 	void			(*rs485_stop_tx)(struct uart_8250_port *up, bool toggle_ier);
 

-- 
2.56.0