tty: tty_port: add workqueue to flip TTY buffer

On the embedded platform, certain critical data, such as IMU data, is
transmitted through UART. The tty_flip_buffer_push() interface in the TTY
layer uses system_dfl_wq to handle the flipping of the TTY buffer.
Although the unbound workqueue can create new threads on demand and wake
up the kworker thread on an idle CPU, it may be preempted by real-time
tasks or other high-prio tasks.

flush_to_ldisc() needs to wake up the relevant data handle thread. When
executing __wake_up_common_lock(), it calls spin_lock_irqsave(), which
does not disable preemption but disables migration in RT-Linux. This
prevents the kworker thread from being migrated to other cores by CPU's
balancing logic, resulting in long delays. The call trace is as follows:
    __wake_up_common_lock
    __wake_up
    ep_poll_callback
    __wake_up_common
    __wake_up_common_lock
    __wake_up
    n_tty_receive_buf_common
    n_tty_receive_buf2
    tty_ldisc_receive_buf
    tty_port_default_receive_buf
    flush_to_ldisc

In our system, the processing interval for each frame of IMU data
transmitted via UART can experience significant jitter due to this issue.
Instead of the expected 10 to 15 ms frame processing interval, we see
spikes up to 30 to 35 ms. Moreover, in just one or two hours, there can
be 2 to 3 occurrences of such high jitter, which is quite frequent. This
jitter exceeds the software's tolerable limit of 20 ms.

Introduce flip_wq in tty_port which can be set by tty_port_link_wq() or as
default linked to default workqueue allocated when tty_register_driver().
The default workqueue is allocated with flag WQ_SYSFS, so that cpumask and
nice can be set dynamically. The execution timing of tty_port_link_wq() is
not clearly restricted. The newly added function tty_port_link_driver_wq()
checks whether the flip_wq of the tty_port has already been assigned when
linking the default tty_driver's workqueue to the port. After the user has
set a custom workqueue for a certain tty_port using tty_port_link_wq(), the
system will only use this custom workqueue, even if tty_driver does not
have %TTY_DRIVER_CUSTOM_WORKQUEUE flag.

Introduce %TTY_DRIVER_CUSTOM_WORKQUEUE flag meaning not to create the
default single tty_driver workqueue. Two reasons why need to introduce the
%TTY_DRIVER_CUSTOM_WORKQUEUE flag:
1. If the WQ_SYSFS parameter is enabled, workqueue_sysfs_register() will
fail when trying to create a workqueue with the same name. The pty is an
example of this; if both CONFIG_LEGACY_PTYS and CONFIG_UNIX98_PTYS are
enabled, the call to tty_register_driver() in unix98_pty_init() will fail.
2. Different tty ports may be used for different tasks, which may require
separate core binding control via workqueues. In this case, the workqueue
created by default in the tty driver is unnecessary. Enabling this flag
prevents the creation of this redundant workqueue.

After applying this patch, we can set the related UART TTY flip buffer
workqueue by sysfs. We set the cpumask to CPU cores associated with the
IMU tasks, and set the nice to -20. Testing has shown significant
improvement in the previously described issue, with almost no stuttering
occurring anymore.

Signed-off-by: Xin Zhao <jackzxcui1989@163.com>
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Link: https://patch.msgid.link/20251223034836.2625547-1-jackzxcui1989@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 6120d827a797747233859ec740e89a6b8634930b..1f17575f8fe0c53de88791de6fb8aacec0ed6ef9 100644 (file)
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -403,6 +403,8 @@ static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty,
        o_tty->link = tty;
        tty_port_init(ports[0]);
        tty_port_init(ports[1]);
+       tty_port_link_wq(ports[0], system_dfl_wq);
+       tty_port_link_wq(ports[1], system_dfl_wq);
        tty_buffer_set_limit(ports[0], 8192);
        tty_buffer_set_limit(ports[1], 8192);
        o_tty->port = ports[0];
@@ -532,14 +534,16 @@ static void __init legacy_pty_init(void)
        pty_driver = tty_alloc_driver(legacy_count,
                        TTY_DRIVER_RESET_TERMIOS |
                        TTY_DRIVER_REAL_RAW |
-                       TTY_DRIVER_DYNAMIC_ALLOC);
+                       TTY_DRIVER_DYNAMIC_ALLOC |
+                       TTY_DRIVER_CUSTOM_WORKQUEUE);
        if (IS_ERR(pty_driver))
                panic("Couldn't allocate pty driver");
 
        pty_slave_driver = tty_alloc_driver(legacy_count,
                        TTY_DRIVER_RESET_TERMIOS |
                        TTY_DRIVER_REAL_RAW |
-                       TTY_DRIVER_DYNAMIC_ALLOC);
+                       TTY_DRIVER_DYNAMIC_ALLOC |
+                       TTY_DRIVER_CUSTOM_WORKQUEUE);
        if (IS_ERR(pty_slave_driver))
                panic("Couldn't allocate pty slave driver");
 
@@ -849,7 +853,8 @@ static void __init unix98_pty_init(void)
                        TTY_DRIVER_REAL_RAW |
                        TTY_DRIVER_DYNAMIC_DEV |
                        TTY_DRIVER_DEVPTS_MEM |
-                       TTY_DRIVER_DYNAMIC_ALLOC);
+                       TTY_DRIVER_DYNAMIC_ALLOC |
+                       TTY_DRIVER_CUSTOM_WORKQUEUE);
        if (IS_ERR(ptm_driver))
                panic("Couldn't allocate Unix98 ptm driver");
        pts_driver = tty_alloc_driver(NR_UNIX98_PTY_MAX,
@@ -857,7 +862,8 @@ static void __init unix98_pty_init(void)
                        TTY_DRIVER_REAL_RAW |
                        TTY_DRIVER_DYNAMIC_DEV |
                        TTY_DRIVER_DEVPTS_MEM |
-                       TTY_DRIVER_DYNAMIC_ALLOC);
+                       TTY_DRIVER_DYNAMIC_ALLOC |
+                       TTY_DRIVER_CUSTOM_WORKQUEUE);
        if (IS_ERR(pts_driver))
                panic("Couldn't allocate Unix98 pts driver");
 

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 1a5673acd9b107d55623defcfc1dadb853781274..86e1e7178e9001dda2f94be7c83d164d291aa1a6 100644 (file)
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -76,7 +76,7 @@ void tty_buffer_unlock_exclusive(struct tty_port *port)
        mutex_unlock(&buf->lock);
 
        if (restart)
-               queue_work(system_dfl_wq, &buf->work);
+               queue_work(buf->flip_wq, &buf->work);
 }
 EXPORT_SYMBOL_GPL(tty_buffer_unlock_exclusive);
 
@@ -530,7 +530,7 @@ void tty_flip_buffer_push(struct tty_port *port)
        struct tty_bufhead *buf = &port->buf;
 
        tty_flip_buffer_commit(buf->tail);
-       queue_work(system_dfl_wq, &buf->work);
+       queue_work(buf->flip_wq, &buf->work);
 }
 EXPORT_SYMBOL(tty_flip_buffer_push);
 
@@ -560,7 +560,7 @@ int tty_insert_flip_string_and_push_buffer(struct tty_port *port,
                tty_flip_buffer_commit(buf->tail);
        spin_unlock_irqrestore(&port->lock, flags);
 
-       queue_work(system_dfl_wq, &buf->work);
+       queue_work(buf->flip_wq, &buf->work);
 
        return size;
 }
@@ -613,7 +613,7 @@ void tty_buffer_set_lock_subclass(struct tty_port *port)
 
 bool tty_buffer_restart_work(struct tty_port *port)
 {
-       return queue_work(system_dfl_wq, &port->buf.work);
+       return queue_work(port->buf.flip_wq, &port->buf.work);
 }
 
 bool tty_buffer_cancel_work(struct tty_port *port)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e2d92cf70eb78ec6b2b93b55192e46781160c9dc..d64fb08baa179cfd63bd10ba6e624f2f5d41d43f 100644 (file)
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3446,10 +3446,23 @@ int tty_register_driver(struct tty_driver *driver)
        if (error < 0)
                goto err;
 
+       if (!(driver->flags & TTY_DRIVER_CUSTOM_WORKQUEUE)) {
+               driver->flip_wq = alloc_workqueue("%s-flip-wq", WQ_UNBOUND | WQ_SYSFS,
+                                                 0, driver->name);
+               if (!driver->flip_wq) {
+                       error = -ENOMEM;
+                       goto err_unreg_char;
+               }
+               for (i = 0; i < driver->num; i++) {
+                       if (driver->ports[i])
+                               tty_port_link_driver_wq(driver->ports[i], driver);
+               }
+       }
+
        if (driver->flags & TTY_DRIVER_DYNAMIC_ALLOC) {
                error = tty_cdev_add(driver, dev, 0, driver->num);
                if (error)
-                       goto err_unreg_char;
+                       goto err_destroy_wq;
        }
 
        scoped_guard(mutex, &tty_mutex)
@@ -3475,6 +3488,10 @@ err_unreg_devs:
        scoped_guard(mutex, &tty_mutex)
                list_del(&driver->tty_drivers);
 
+err_destroy_wq:
+       if (!(driver->flags & TTY_DRIVER_CUSTOM_WORKQUEUE))
+               destroy_workqueue(driver->flip_wq);
+
 err_unreg_char:
        unregister_chrdev_region(dev, driver->num);
 err:
@@ -3494,6 +3511,8 @@ void tty_unregister_driver(struct tty_driver *driver)
                                driver->num);
        scoped_guard(mutex, &tty_mutex)
                list_del(&driver->tty_drivers);
+       if (!(driver->flags & TTY_DRIVER_CUSTOM_WORKQUEUE))
+               destroy_workqueue(driver->flip_wq);
 }
 EXPORT_SYMBOL(tty_unregister_driver);
 

diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index fe67c5cb0a3f0c36c0581b44ea21ba858e8a2c4d..611f878149f8a0eb052e7c8814d4044287aab0de 100644 (file)
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -99,6 +99,26 @@ void tty_port_init(struct tty_port *port)
 }
 EXPORT_SYMBOL(tty_port_init);
 
+/**
+ * tty_port_link_wq - link tty_port and flip workqueue
+ * @port: tty_port of the device
+ * @flip_wq: workqueue to queue flip buffer work on
+ *
+ * When %TTY_DRIVER_CUSTOM_WORKQUEUE is used, every tty_port shall be linked to
+ * a workqueue manually by this function, otherwise tty_flip_buffer_push() will
+ * see %NULL flip_wq pointer on queue_work.
+ * When %TTY_DRIVER_CUSTOM_WORKQUEUE is NOT used, the function can be used to
+ * link a certain port to a specific workqueue, instead of using the workqueue
+ * allocated in tty_register_driver().
+ *
+ * Note that TTY port API will NOT destroy the workqueue.
+ */
+void tty_port_link_wq(struct tty_port *port, struct workqueue_struct *flip_wq)
+{
+       port->buf.flip_wq = flip_wq;
+}
+EXPORT_SYMBOL_GPL(tty_port_link_wq);
+
 /**
  * tty_port_link_device - link tty and tty_port
  * @port: tty_port of the device
@@ -157,6 +177,7 @@ struct device *tty_port_register_device_attr(struct tty_port *port,
                const struct attribute_group **attr_grp)
 {
        tty_port_link_device(port, driver, index);
+       tty_port_link_driver_wq(port, driver);
        return tty_register_device_attr(driver, index, device, drvdata,
                        attr_grp);
 }
@@ -183,6 +204,7 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
        struct device *dev;
 
        tty_port_link_device(port, driver, index);
+       tty_port_link_driver_wq(port, driver);
 
        dev = serdev_tty_port_register(port, host, parent, driver, index);
        if (PTR_ERR(dev) != -ENODEV) {
@@ -703,6 +725,7 @@ int tty_port_install(struct tty_port *port, struct tty_driver *driver,
                struct tty_struct *tty)
 {
        tty->port = port;
+       tty_port_link_driver_wq(port, driver);
        return tty_standard_install(driver, tty);
 }
 EXPORT_SYMBOL_GPL(tty_port_install);

diff --git a/include/linux/tty_buffer.h b/include/linux/tty_buffer.h
index 31125e3be3c55e4e65dd62d91fd13e96a91e6c27..48adcb0e8ff390c8b8f20292c05ca8262d87b95c 100644 (file)
--- a/include/linux/tty_buffer.h
+++ b/include/linux/tty_buffer.h
@@ -34,6 +34,7 @@ static inline u8 *flag_buf_ptr(struct tty_buffer *b, unsigned int ofs)
 
 struct tty_bufhead {
        struct tty_buffer *head;        /* Queue head */
+       struct workqueue_struct *flip_wq;
        struct work_struct work;
        struct mutex       lock;
        atomic_t           priority;

diff --git a/include/linux/tty_driver.h b/include/linux/tty_driver.h
index 188ee9b768eb6d928c3c5ae2fd1811755c529e62..9c65854f7d94e729e1d95f99ba75c5c1cf2ccd66 100644 (file)
--- a/include/linux/tty_driver.h
+++ b/include/linux/tty_driver.h
@@ -69,6 +69,10 @@ struct serial_struct;
  *     Do not create numbered ``/dev`` nodes. For example, create
  *     ``/dev/ttyprintk`` and not ``/dev/ttyprintk0``. Applicable only when a
  *     driver for a single tty device is being allocated.
+ *
+ * @TTY_DRIVER_CUSTOM_WORKQUEUE:
+ *     Do not create workqueue when tty_register_driver(). When set, flip
+ *     buffer workqueue shall be set by tty_port_link_wq() for every port.
  */
 enum tty_driver_flag {
        TTY_DRIVER_INSTALLED            = BIT(0),
@@ -79,6 +83,7 @@ enum tty_driver_flag {
        TTY_DRIVER_HARDWARE_BREAK       = BIT(5),
        TTY_DRIVER_DYNAMIC_ALLOC        = BIT(6),
        TTY_DRIVER_UNNUMBERED_NODE      = BIT(7),
+       TTY_DRIVER_CUSTOM_WORKQUEUE     = BIT(8),
 };
 
 enum tty_driver_type {
@@ -506,6 +511,7 @@ struct tty_operations {
  * @flags: tty driver flags (%TTY_DRIVER_)
  * @proc_entry: proc fs entry, used internally
  * @other: driver of the linked tty; only used for the PTY driver
+ * @flip_wq: workqueue to queue flip buffer work on
  * @ttys: array of active &struct tty_struct, set by tty_standard_install()
  * @ports: array of &struct tty_port; can be set during initialization by
  *        tty_port_link_device() and similar
@@ -539,6 +545,7 @@ struct tty_driver {
        unsigned long   flags;
        struct proc_dir_entry *proc_entry;
        struct tty_driver *other;
+       struct workqueue_struct *flip_wq;
 
        /*
         * Pointer to the tty data structures

diff --git a/include/linux/tty_port.h b/include/linux/tty_port.h
index 660c254f1efe5f61f41244f2bb8cfa6bec5f4849..c1b87f3c56034a5ec801eede1660a7998757b486 100644 (file)
--- a/include/linux/tty_port.h
+++ b/include/linux/tty_port.h
@@ -138,6 +138,7 @@ struct tty_port {
                                           kernel */
 
 void tty_port_init(struct tty_port *port);
+void tty_port_link_wq(struct tty_port *port, struct workqueue_struct *flip_wq);
 void tty_port_link_device(struct tty_port *port, struct tty_driver *driver,
                unsigned index);
 struct device *tty_port_register_device(struct tty_port *port,
@@ -165,6 +166,18 @@ static inline struct tty_port *tty_port_get(struct tty_port *port)
        return NULL;
 }
 
+/*
+ * Never overwrite the workqueue set by tty_port_link_wq().
+ * No effect when %TTY_DRIVER_CUSTOM_WORKQUEUE is set, as driver->flip_wq is
+ * %NULL.
+ */
+static inline void tty_port_link_driver_wq(struct tty_port *port,
+                                          struct tty_driver *driver)
+{
+       if (!port->buf.flip_wq)
+               port->buf.flip_wq = driver->flip_wq;
+}
+
 /* If the cts flow control is enabled, return true. */
 static inline bool tty_port_cts_enabled(const struct tty_port *port)
 {