[PATCH] Input: serport - defer write wakeups to a worker

[PATCH] Input: serport - defer write wakeups to a worker

[James Hilliard]

UART drivers can invoke the line discipline's write_wakeup() callback with
their port lock held. serport forwards the notification directly to the
serio driver. I-Force's callback can transmit a queued packet through
serio_write(), which calls back into uart_write() and tries to acquire the
same port lock. A notification after I-Force queues a packet but before it
sets IFORCE_XMIT_RUNNING can therefore deadlock.

Defer the serio notification to a work item, as required for transmitting
from a line discipline's write-wakeup path. Keep the SERPORT_ACTIVE check
and callback under serport->lock to serialize with serio close. Cancel the
work before freeing serport in line-discipline close, where the TTY layer
has already excluded further line-discipline callbacks.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/input/serio/serport.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index cabe3876c168..24c0d0c0c9aa 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -18,6 +18,7 @@
 #include <linux/serio.h>
 #include <linux/tty.h>
 #include <linux/compat.h>
+#include <linux/workqueue.h>
 
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@ucw.cz>");
 MODULE_DESCRIPTION("Input device TTY line discipline");
@@ -33,6 +34,7 @@ struct serport {
 	wait_queue_head_t wait;
 	struct serio *serio;
 	struct serio_device_id id;
+	struct work_struct write_wakeup;
 	spinlock_t lock;
 	unsigned long flags;
 };
@@ -66,6 +68,16 @@ static void serport_serio_close(struct serio *serio)
 	clear_bit(SERPORT_ACTIVE, &serport->flags);
 }
 
+static void serport_write_wakeup(struct work_struct *work)
+{
+	struct serport *serport = container_of(work, struct serport, write_wakeup);
+
+	guard(spinlock_irqsave)(&serport->lock);
+
+	if (test_bit(SERPORT_ACTIVE, &serport->flags))
+		serio_drv_write_wakeup(serport->serio);
+}
+
 /*
  * serport_ldisc_open() is the routine that is called upon setting our line
  * discipline on a tty. It prepares the serio struct.
@@ -85,6 +97,7 @@ static int serport_ldisc_open(struct tty_struct *tty)
 	serport->tty = tty;
 	spin_lock_init(&serport->lock);
 	init_waitqueue_head(&serport->wait);
+	INIT_WORK(&serport->write_wakeup, serport_write_wakeup);
 
 	tty->disc_data = serport;
 	tty->receive_room = 256;
@@ -101,6 +114,7 @@ static void serport_ldisc_close(struct tty_struct *tty)
 {
 	struct serport *serport = tty->disc_data;
 
+	cancel_work_sync(&serport->write_wakeup);
 	kfree(serport);
 }
 
@@ -249,10 +263,8 @@ static void serport_ldisc_write_wakeup(struct tty_struct * tty)
 {
 	struct serport *serport = tty->disc_data;
 
-	guard(spinlock_irqsave)(&serport->lock);
-
-	if (test_bit(SERPORT_ACTIVE, &serport->flags))
-		serio_drv_write_wakeup(serport->serio);
+	/* A serio driver may write to the TTY, so defer the callback. */
+	schedule_work(&serport->write_wakeup);
 }
 
 /*

---
base-commit: 2e6f2e5799f328ca0879dad14d7f3709c7f7496b
change-id: 20261023-submit-serport-write-wakeup-v1-1bdd1006fafa

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

Re: [PATCH] Input: serport - defer write wakeups to a worker

[Dmitry Torokhov]

Hi James,

On Fri, Oct 23, 2026 at 10:23:44PM -0600, James Hilliard wrote:
> Defer the serio notification to a work item, as required for transmitting
> from a line discipline's write-wakeup path. Keep the SERPORT_ACTIVE check
> and callback under serport->lock to serialize with serio close. Cancel the
> work before freeing serport in line-discipline close, where the TTY layer
> has already excluded further line-discipline callbacks.
...
> @@ -66,6 +68,16 @@ static void serport_serio_close(struct serio *serio)
>  	clear_bit(SERPORT_ACTIVE, &serport->flags);
>  }
>  
> +static void serport_write_wakeup(struct work_struct *work)
> +{
> +	struct serport *serport = container_of(work, struct serport, write_wakeup);
> +
> +	guard(spinlock_irqsave)(&serport->lock);
> +
> +	if (test_bit(SERPORT_ACTIVE, &serport->flags))
> +		serio_drv_write_wakeup(serport->serio);
> +}
> +
>  /*
>   * serport_ldisc_open() is the routine that is called upon setting our line
>   * discipline on a tty. It prepares the serio struct.
> @@ -85,6 +97,7 @@ static int serport_ldisc_open(struct tty_struct *tty)
>  	serport->tty = tty;
>  	spin_lock_init(&serport->lock);
>  	init_waitqueue_head(&serport->wait);
> +	INIT_WORK(&serport->write_wakeup, serport_write_wakeup);
>  
>  	tty->disc_data = serport;
>  	tty->receive_room = 256;
> @@ -101,6 +114,7 @@ static void serport_ldisc_close(struct tty_struct *tty)
>  {
>  	struct serport *serport = tty->disc_data;
>  
> +	cancel_work_sync(&serport->write_wakeup);
>  	kfree(serport);
>  }
>  
> @@ -249,10 +263,8 @@ static void serport_ldisc_write_wakeup(struct tty_struct * tty)
>  {
>  	struct serport *serport = tty->disc_data;
>  
> -	guard(spinlock_irqsave)(&serport->lock);
> -
> -	if (test_bit(SERPORT_ACTIVE, &serport->flags))
> -		serio_drv_write_wakeup(serport->serio);
> +	/* A serio driver may write to the TTY, so defer the callback. */
> +	schedule_work(&serport->write_wakeup);
>  }

Thank you for the patch. Deferring write_wakeup() in serport makes sense
(it also breaks a port->lock -> serport->lock -> input_dev->event_lock ->
port->lock lock ordering cycle with input event callbacks that call
serio_write()), however could we tighten the lifecycle and avoid scheduling
work when the bound serio driver does not implement write_wakeup()?

Right now serport_ldisc_open() sets TTY_DO_WRITE_WAKEUP unconditionally
(and never clears it), which means tty_wakeup() will invoke
serport_ldisc_write_wakeup() and schedule work on every TX wakeup even when
no serio driver is bound or the driver has no write_wakeup() callback
(every driver except iforce).

Instead, we can initialize serport->write_wakeup in serport_ldisc_open()
and immediately disable it with disable_work(), without setting
TTY_DO_WRITE_WAKEUP there. Then in serport_serio_open(), if
serio->drv->write_wakeup is set, call enable_work(&serport->write_wakeup)
and set TTY_DO_WRITE_WAKEUP on serport->tty->flags; and in
serport_serio_close(), if serio->drv->write_wakeup is set, clear
TTY_DO_WRITE_WAKEUP and call disable_work_sync(&serport->write_wakeup).

With serport_serio_close() draining the work before serio->drv or
serport->serio is torn down, serport_write_wakeup() will not even need to
acquire serport->lock or check SERPORT_ACTIVE before calling
serio_drv_write_wakeup(serport->serio).

Also, in serport_ldisc_hangup(), please clear TTY_DO_WRITE_WAKEUP and call
disable_work_sync(&serport->write_wakeup). tty_ldisc_hangup() explicitly
invokes ld->ops->write_wakeup() right before ld->ops->hangup(), while
TTY_DO_WRITE_WAKEUP and low-level driver TX wakeups remain active until
after tty_ldisc_hangup() finishes waiting for serport_ldisc_read() to
unregister the serio port. Disabling and draining the work in hangup
ensures we stop TX wakeups immediately on hangup and keep the work disabled
even if an asynchronous driver probe races with hangup.

Thanks.

-- 
Dmitry