OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

serial: change retry logic to avoid concurrency

Browse Source 
Whenever serial_xmit fails to transmit a byte it adds a watch that would
call it again when the "line" becomes ready. This results in a retry
chain:
  serial_xmit -> add_watch -> serial_xmit
Each chain is able to transmit one character, and for every character
passed to serial by the guest driver a new chain is spawned.

The problem lays with the fact that a new chain is spawned even when
there is one already waiting on the watch. So there can be several retry
chains waiting concurrently on one "line". Every chain tries to transmit
current character, so character order is not messed up. But also every
chain increases retry counter (tsr_retry). If there are enough
concurrent chains this counter will hit MAX_XMIT_RETRY value and
the character will be dropped.

To reproduce this bug you need to feed serial output to some program
consuming it slowly enough. A python script from bug #1335444
description is an example of such program.

This commit changes retry logic in the following way to avoid
concurrency: instead of spawning a new chain for each character being
transmitted spawn only one and make it transmit characters until FIFO is
empty.

The change consists of two parts:
 - add a do {} while () loop in serial_xmit (diff is a bit erratic
   for this part, diff -w will show actual change),
 - do not call serial_xmit from serial_ioport_write if there is one
   waiting on the watch already.

This should fix another issue causing bug #1335444.

Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Kirill Batuzov 2014-07-11 13:41:08 +04:00 committed by Paolo Bonzini
parent 7b3621f47a
commit f702e62a19
1 changed files with 35 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

63
hw/char/serial.c
View File

@ -223,37 +223,42 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
{
SerialState *s = opaque;
if (s->tsr_retry <= 0) {
if (s->fcr & UART_FCR_FE) {
if (fifo8_is_empty(&s->xmit_fifo)) {
do {
if (s->tsr_retry <= 0) {
if (s->fcr & UART_FCR_FE) {
if (fifo8_is_empty(&s->xmit_fifo)) {
return FALSE;
}
s->tsr = fifo8_pop(&s->xmit_fifo);
if (!s->xmit_fifo.num) {
s->lsr |= UART_LSR_THRE;
}
} else if ((s->lsr & UART_LSR_THRE)) {
return FALSE;
} else {
s->tsr = s->thr;
s->lsr |= UART_LSR_THRE;
s->lsr &= ~UART_LSR_TEMT;
}
}
if (s->mcr & UART_MCR_LOOP) {
/* in loopback mode, say that we just received a char */
serial_receive1(s, &s->tsr, 1);
} else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP,
serial_xmit, s) > 0) {
s->tsr_retry++;
return FALSE;
}
s->tsr = fifo8_pop(&s->xmit_fifo);
if (!s->xmit_fifo.num) {
s->lsr |= UART_LSR_THRE;
}
} else if ((s->lsr & UART_LSR_THRE)) {
return FALSE;
s->tsr_retry = 0;
} else {
s->tsr = s->thr;
s->lsr |= UART_LSR_THRE;
s->lsr &= ~UART_LSR_TEMT;
s->tsr_retry = 0;
}
}
if (s->mcr & UART_MCR_LOOP) {
/* in loopback mode, say that we just received a char */
serial_receive1(s, &s->tsr, 1);
} else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
if (s->tsr_retry >= 0 && s->tsr_retry < MAX_XMIT_RETRY &&
qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP, serial_xmit, s) > 0) {
s->tsr_retry++;
return FALSE;
}
s->tsr_retry = 0;
} else {
s->tsr_retry = 0;
}
/* Transmit another byte if it is already available. It is only
possible when FIFO is enabled and not empty. */
} while ((s->fcr & UART_FCR_FE) && !fifo8_is_empty(&s->xmit_fifo));
s->last_xmit_ts = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
@ -293,7 +298,9 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
s->thr_ipending = 0;
s->lsr &= ~UART_LSR_THRE;
serial_update_irq(s);
serial_xmit(NULL, G_IO_OUT, s);
if (s->tsr_retry <= 0) {
serial_xmit(NULL, G_IO_OUT, s);
}
}
break;
case 1: