Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

tty: Fix low_latency BUG

The user-settable knob, low_latency, has been the source of
several BUG reports which stem from flush_to_ldisc() running
in interrupt context. Since 3.12, which added several sleeping
locks (termios_rwsem and buf->lock) to the input processing path,
the frequency of these BUG reports has increased.

Note that changes in 3.12 did not introduce this regression;
sleeping locks were first added to the input processing path
with the removal of the BKL from N_TTY in commit
a88a69c91256418c5907c2f1f8a0ec0a36f9e6cc,
'n_tty: Fix loss of echoed characters and remove bkl from n_tty'
and later in commit 38db89799bdf11625a831c5af33938dcb11908b6,
'tty: throttling race fix'. Since those changes, executing
flush_to_ldisc() in interrupt_context (ie, low_latency set), is unsafe.

However, since most devices do not validate if the low_latency
setting is appropriate for the context (process or interrupt) in
which they receive data, some reports are due to misconfiguration.
Further, serial dma devices for which dma fails, resort to
interrupt receiving as a backup without resetting low_latency.

Historically, low_latency was used to force wake-up the reading
process rather than wait for the next scheduler tick. The
effect was to trim multiple milliseconds of latency from
when the process would receive new data.

Recent tests [1] have shown that the reading process now receives
data with only 10's of microseconds latency without low_latency set.

Remove the low_latency rx steering from tty_flip_buffer_push();
however, leave the knob as an optional hint to drivers that can
tune their rx fifos and such like. Cleanup stale code comments
regarding low_latency.

[1] https://lkml.org/lkml/2014/2/20/434

"Yay.. thats an annoying historical pain in the butt gone."
-- Alan Cox

Reported-by: Beat Bolli <bbolli@ewanet.ch>
Reported-by: Pavel Roskin <proski@gnu.org>
Acked-by: David Sterba <dsterba@suse.cz>
Cc: Grant Edwards <grant.b.edwards@gmail.com>
Cc: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Hal Murray <murray+fedora@ip-64-139-1-69.sjc.megapath.net>
Cc: <stable@vger.kernel.org> # 3.12.x+
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Peter Hurley and committed by
Greg Kroah-Hartman
a9c3f68f 9277285f

+7 -22
-3
drivers/tty/ipwireless/tty.c
··· 176 176 ": %d chars not inserted to flip buffer!\n", 177 177 length - work); 178 178 179 - /* 180 - * This may sleep if ->low_latency is set 181 - */ 182 179 if (work) 183 180 tty_flip_buffer_push(&tty->port); 184 181 }
+4 -16
drivers/tty/tty_buffer.c
··· 351 351 * Takes any pending buffers and transfers their ownership to the 352 352 * ldisc side of the queue. It then schedules those characters for 353 353 * processing by the line discipline. 354 - * Note that this function can only be used when the low_latency flag 355 - * is unset. Otherwise the workqueue won't be flushed. 356 354 */ 357 355 358 356 void tty_schedule_flip(struct tty_port *port) 359 357 { 360 358 struct tty_bufhead *buf = &port->buf; 361 - WARN_ON(port->low_latency); 362 359 363 360 buf->tail->commit = buf->tail->used; 364 361 schedule_work(&buf->work); ··· 479 482 */ 480 483 void tty_flush_to_ldisc(struct tty_struct *tty) 481 484 { 482 - if (!tty->port->low_latency) 483 - flush_work(&tty->port->buf.work); 485 + flush_work(&tty->port->buf.work); 484 486 } 485 487 486 488 /** 487 489 * tty_flip_buffer_push - terminal 488 490 * @port: tty port to push 489 491 * 490 - * Queue a push of the terminal flip buffers to the line discipline. This 491 - * function must not be called from IRQ context if port->low_latency is 492 - * set. 492 + * Queue a push of the terminal flip buffers to the line discipline. 493 + * Can be called from IRQ/atomic context. 493 494 * 494 495 * In the event of the queue being busy for flipping the work will be 495 496 * held off and retried later. ··· 495 500 496 501 void tty_flip_buffer_push(struct tty_port *port) 497 502 { 498 - struct tty_bufhead *buf = &port->buf; 499 - 500 - buf->tail->commit = buf->tail->used; 501 - 502 - if (port->low_latency) 503 - flush_to_ldisc(&buf->work); 504 - else 505 - schedule_work(&buf->work); 503 + tty_schedule_flip(port); 506 504 } 507 505 EXPORT_SYMBOL(tty_flip_buffer_push); 508 506
+2 -2
drivers/usb/gadget/u_serial.c
··· 549 549 port->read_started--; 550 550 } 551 551 552 - /* Push from tty to ldisc; without low_latency set this is handled by 553 - * a workqueue, so we won't get callbacks and can hold port_lock 552 + /* Push from tty to ldisc; this is handled by a workqueue, 553 + * so we won't get callbacks and can hold port_lock 554 554 */ 555 555 if (do_push) 556 556 tty_flip_buffer_push(&port->port);
+1 -1
include/linux/tty.h
··· 208 208 wait_queue_head_t delta_msr_wait; /* Modem status change */ 209 209 unsigned long flags; /* TTY flags ASY_*/ 210 210 unsigned char console:1, /* port is a console */ 211 - low_latency:1; /* direct buffer flush */ 211 + low_latency:1; /* optional: tune for latency */ 212 212 struct mutex mutex; /* Locking */ 213 213 struct mutex buf_mutex; /* Buffer alloc lock */ 214 214 unsigned char *xmit_buf; /* Optional buffer */