Skip to content

Commit eb3b0d9

Browse files
jackzxcui1989gregkh
authored andcommitted
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_NO_WORKQUEUE flag. When tty_port register device, flip_wq link operation is done by tty_port_link_driver_wq(), but for in-memory devices the link operation cannot cover all the cases. Although tty_port_install() is dedicated for in-memory devices lik PTY to link port allocated on demand, the logic of tty_port_install() is so simple that people may not call it, vc_cons[0].d->port is one such case. We check the buf.flip_wq when flip TTY buffer, if buf.flip_wq of TTY port is NULL, use system_dfl_wq as a backup. To avoid naming conflict of the default tty_driver's workqueue, using '"%s-%s", driver->name, driver->driver_name' as the workqueue name. In cases where driver_name is not specified and therefore is NULL, the workqueue is not created. Drivers that do not define driver_name are potentially in-memory devices like vty, which generally do not require special workqueue settings. Even with the combination of name and driver_name, the workqueue names can still be duplicated, as many tty serial drivers use "ttyS" as dev_name and "serial" as driver_name. I modified the conflicting driver_name of these drivers by appending a suffix of _xx based on the corresponding .c file. If this modification is not made, it could not only lead to duplicate workqueue names but also result in duplicate entries for the /proc/tty/driver/<driver_name> nodes. Introduce %TTY_DRIVER_NO_WORKQUEUE flag meaning not to create the default single tty_driver workqueue. Two reasons why need to introduce the %TTY_DRIVER_NO_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. Tested-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Xin Zhao <jackzxcui1989@163.com> Link: https://patch.msgid.link/20260213085039.3274704-1-jackzxcui1989@163.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent fa4268c commit eb3b0d9

12 files changed

Lines changed: 91 additions & 14 deletions

File tree

drivers/tty/pty.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -532,14 +532,16 @@ static void __init legacy_pty_init(void)
532532
pty_driver = tty_alloc_driver(legacy_count,
533533
TTY_DRIVER_RESET_TERMIOS |
534534
TTY_DRIVER_REAL_RAW |
535-
TTY_DRIVER_DYNAMIC_ALLOC);
535+
TTY_DRIVER_DYNAMIC_ALLOC |
536+
TTY_DRIVER_NO_WORKQUEUE);
536537
if (IS_ERR(pty_driver))
537538
panic("Couldn't allocate pty driver");
538539

539540
pty_slave_driver = tty_alloc_driver(legacy_count,
540541
TTY_DRIVER_RESET_TERMIOS |
541542
TTY_DRIVER_REAL_RAW |
542-
TTY_DRIVER_DYNAMIC_ALLOC);
543+
TTY_DRIVER_DYNAMIC_ALLOC |
544+
TTY_DRIVER_NO_WORKQUEUE);
543545
if (IS_ERR(pty_slave_driver))
544546
panic("Couldn't allocate pty slave driver");
545547

@@ -849,15 +851,17 @@ static void __init unix98_pty_init(void)
849851
TTY_DRIVER_REAL_RAW |
850852
TTY_DRIVER_DYNAMIC_DEV |
851853
TTY_DRIVER_DEVPTS_MEM |
852-
TTY_DRIVER_DYNAMIC_ALLOC);
854+
TTY_DRIVER_DYNAMIC_ALLOC |
855+
TTY_DRIVER_NO_WORKQUEUE);
853856
if (IS_ERR(ptm_driver))
854857
panic("Couldn't allocate Unix98 ptm driver");
855858
pts_driver = tty_alloc_driver(NR_UNIX98_PTY_MAX,
856859
TTY_DRIVER_RESET_TERMIOS |
857860
TTY_DRIVER_REAL_RAW |
858861
TTY_DRIVER_DYNAMIC_DEV |
859862
TTY_DRIVER_DEVPTS_MEM |
860-
TTY_DRIVER_DYNAMIC_ALLOC);
863+
TTY_DRIVER_DYNAMIC_ALLOC |
864+
TTY_DRIVER_NO_WORKQUEUE);
861865
if (IS_ERR(pts_driver))
862866
panic("Couldn't allocate Unix98 pts driver");
863867

drivers/tty/serial/8250/8250_core.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,7 @@ console_initcall(univ8250_console_init);
524524

525525
struct uart_driver serial8250_reg = {
526526
.owner = THIS_MODULE,
527-
.driver_name = "serial",
527+
.driver_name = "serial_8250",
528528
.dev_name = "ttyS",
529529
.major = TTY_MAJOR,
530530
.minor = 64,

drivers/tty/serial/apbuart.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ console_initcall(apbuart_console_init);
505505

506506
static struct uart_driver grlib_apbuart_driver = {
507507
.owner = THIS_MODULE,
508-
.driver_name = "serial",
508+
.driver_name = "serial_apbuart",
509509
.dev_name = "ttyS",
510510
.major = SERIAL_APBUART_MAJOR,
511511
.minor = SERIAL_APBUART_MINOR,

drivers/tty/serial/dz.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -914,7 +914,7 @@ console_initcall(dz_serial_console_init);
914914

915915
static struct uart_driver dz_reg = {
916916
.owner = THIS_MODULE,
917-
.driver_name = "serial",
917+
.driver_name = "serial_dz",
918918
.dev_name = "ttyS",
919919
.major = TTY_MAJOR,
920920
.minor = 64,

drivers/tty/serial/ip22zilog.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ static struct console ip22zilog_console = {
10151015

10161016
static struct uart_driver ip22zilog_reg = {
10171017
.owner = THIS_MODULE,
1018-
.driver_name = "serial",
1018+
.driver_name = "serial_ip22zilog",
10191019
.dev_name = "ttyS",
10201020
.major = TTY_MAJOR,
10211021
.minor = 64,

drivers/tty/serial/zs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1252,7 +1252,7 @@ console_initcall(zs_serial_console_init);
12521252

12531253
static struct uart_driver zs_reg = {
12541254
.owner = THIS_MODULE,
1255-
.driver_name = "serial",
1255+
.driver_name = "serial_zs",
12561256
.dev_name = "ttyS",
12571257
.major = TTY_MAJOR,
12581258
.minor = 64,

drivers/tty/tty_buffer.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,13 @@ void tty_buffer_lock_exclusive(struct tty_port *port)
5959
}
6060
EXPORT_SYMBOL_GPL(tty_buffer_lock_exclusive);
6161

62+
static bool tty_buffer_queue_work(struct tty_bufhead *buf)
63+
{
64+
struct workqueue_struct *flip_wq = READ_ONCE(buf->flip_wq);
65+
66+
return queue_work(flip_wq ?: system_dfl_wq, &buf->work);
67+
}
68+
6269
/**
6370
* tty_buffer_unlock_exclusive - release exclusive access
6471
* @port: tty port owning the flip buffer
@@ -76,7 +83,7 @@ void tty_buffer_unlock_exclusive(struct tty_port *port)
7683
mutex_unlock(&buf->lock);
7784

7885
if (restart)
79-
queue_work(system_dfl_wq, &buf->work);
86+
tty_buffer_queue_work(buf);
8087
}
8188
EXPORT_SYMBOL_GPL(tty_buffer_unlock_exclusive);
8289

@@ -530,7 +537,7 @@ void tty_flip_buffer_push(struct tty_port *port)
530537
struct tty_bufhead *buf = &port->buf;
531538

532539
tty_flip_buffer_commit(buf->tail);
533-
queue_work(system_dfl_wq, &buf->work);
540+
tty_buffer_queue_work(buf);
534541
}
535542
EXPORT_SYMBOL(tty_flip_buffer_push);
536543

@@ -560,7 +567,7 @@ int tty_insert_flip_string_and_push_buffer(struct tty_port *port,
560567
tty_flip_buffer_commit(buf->tail);
561568
spin_unlock_irqrestore(&port->lock, flags);
562569

563-
queue_work(system_dfl_wq, &buf->work);
570+
tty_buffer_queue_work(buf);
564571

565572
return size;
566573
}
@@ -613,7 +620,7 @@ void tty_buffer_set_lock_subclass(struct tty_port *port)
613620

614621
bool tty_buffer_restart_work(struct tty_port *port)
615622
{
616-
return queue_work(system_dfl_wq, &port->buf.work);
623+
return tty_buffer_queue_work(&port->buf);
617624
}
618625

619626
bool tty_buffer_cancel_work(struct tty_port *port)

drivers/tty/tty_io.c

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3443,10 +3443,27 @@ int tty_register_driver(struct tty_driver *driver)
34433443
if (error < 0)
34443444
goto err;
34453445

3446+
/*
3447+
* Drivers that do not define driver_name are potentially in-memory devices
3448+
* like vty, which generally do not require special workqueue settings.
3449+
*/
3450+
if (!(driver->flags & TTY_DRIVER_NO_WORKQUEUE) && driver->driver_name) {
3451+
driver->flip_wq = alloc_workqueue("%s-%s", WQ_UNBOUND | WQ_SYSFS,
3452+
0, driver->name, driver->driver_name);
3453+
if (!driver->flip_wq) {
3454+
error = -ENOMEM;
3455+
goto err_unreg_char;
3456+
}
3457+
for (i = 0; i < driver->num; i++) {
3458+
if (driver->ports[i])
3459+
tty_port_link_driver_wq(driver->ports[i], driver);
3460+
}
3461+
}
3462+
34463463
if (driver->flags & TTY_DRIVER_DYNAMIC_ALLOC) {
34473464
error = tty_cdev_add(driver, dev, 0, driver->num);
34483465
if (error)
3449-
goto err_unreg_char;
3466+
goto err_destroy_wq;
34503467
}
34513468

34523469
scoped_guard(mutex, &tty_mutex)
@@ -3472,6 +3489,10 @@ int tty_register_driver(struct tty_driver *driver)
34723489
scoped_guard(mutex, &tty_mutex)
34733490
list_del(&driver->tty_drivers);
34743491

3492+
err_destroy_wq:
3493+
if (driver->flip_wq)
3494+
destroy_workqueue(driver->flip_wq);
3495+
34753496
err_unreg_char:
34763497
unregister_chrdev_region(dev, driver->num);
34773498
err:
@@ -3491,6 +3512,8 @@ void tty_unregister_driver(struct tty_driver *driver)
34913512
driver->num);
34923513
scoped_guard(mutex, &tty_mutex)
34933514
list_del(&driver->tty_drivers);
3515+
if (driver->flip_wq)
3516+
destroy_workqueue(driver->flip_wq);
34943517
}
34953518
EXPORT_SYMBOL(tty_unregister_driver);
34963519

drivers/tty/tty_port.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,23 @@ void tty_port_init(struct tty_port *port)
9999
}
100100
EXPORT_SYMBOL(tty_port_init);
101101

102+
/**
103+
* tty_port_link_wq - link tty_port and flip workqueue
104+
* @port: tty_port of the device
105+
* @flip_wq: workqueue to queue flip buffer work on
106+
*
107+
* Whenever %TTY_DRIVER_NO_WORKQUEUE is used, every tty_port can be linked to
108+
* a workqueue manually by this function.
109+
* tty_port will use system_dfl_wq when buf.flip_wq is NULL.
110+
*
111+
* Note that tty_port API will NOT destroy the workqueue.
112+
*/
113+
void tty_port_link_wq(struct tty_port *port, struct workqueue_struct *flip_wq)
114+
{
115+
port->buf.flip_wq = flip_wq;
116+
}
117+
EXPORT_SYMBOL_GPL(tty_port_link_wq);
118+
102119
/**
103120
* tty_port_link_device - link tty and tty_port
104121
* @port: tty_port of the device
@@ -157,6 +174,7 @@ struct device *tty_port_register_device_attr(struct tty_port *port,
157174
const struct attribute_group **attr_grp)
158175
{
159176
tty_port_link_device(port, driver, index);
177+
tty_port_link_driver_wq(port, driver);
160178
return tty_register_device_attr(driver, index, device, drvdata,
161179
attr_grp);
162180
}
@@ -183,6 +201,7 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
183201
struct device *dev;
184202

185203
tty_port_link_device(port, driver, index);
204+
tty_port_link_driver_wq(port, driver);
186205

187206
dev = serdev_tty_port_register(port, host, parent, driver, index);
188207
if (PTR_ERR(dev) != -ENODEV) {
@@ -210,6 +229,7 @@ void tty_port_unregister_device(struct tty_port *port,
210229
{
211230
int ret;
212231

232+
WRITE_ONCE(port->buf.flip_wq, NULL);
213233
ret = serdev_tty_port_unregister(port);
214234
if (ret == 0)
215235
return;
@@ -257,6 +277,7 @@ void tty_port_destroy(struct tty_port *port)
257277
{
258278
tty_buffer_cancel_work(port);
259279
tty_buffer_free_all(port);
280+
WRITE_ONCE(port->buf.flip_wq, NULL);
260281
}
261282
EXPORT_SYMBOL(tty_port_destroy);
262283

@@ -703,6 +724,7 @@ int tty_port_install(struct tty_port *port, struct tty_driver *driver,
703724
struct tty_struct *tty)
704725
{
705726
tty->port = port;
727+
tty_port_link_driver_wq(port, driver);
706728
return tty_standard_install(driver, tty);
707729
}
708730
EXPORT_SYMBOL_GPL(tty_port_install);

include/linux/tty_buffer.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ static inline u8 *flag_buf_ptr(struct tty_buffer *b, unsigned int ofs)
3434

3535
struct tty_bufhead {
3636
struct tty_buffer *head; /* Queue head */
37+
struct workqueue_struct *flip_wq;
3738
struct work_struct work;
3839
struct mutex lock;
3940
atomic_t priority;

0 commit comments

Comments
 (0)