On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote: > From: Josef Bacik > > 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 > Cc: Jiri Slaby > Cc: Andy Shevchenko > Cc: Hugo Villeneuve > Cc: "Uwe Kleine-König (The Capable Hub)" > Cc: "Ilpo Järvinen" > Cc: Tate Whiteberg > Cc: Christian Marangi > Cc: John Ogness > Cc: Crescent Hsieh > Cc: Fushuai Wang > Cc: Rafael Gago Castano > Signed-off-by: Josef Bacik > Signed-off-by: Greg Kroah-Hartman > --- > 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); The name is pretty misleading after this change when all it now does is stop timers. -- i. > > +/** > + * 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); > > >