[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/char/serial: Only retry if qemu_chr_fe_write
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH] hw/char/serial: Only retry if qemu_chr_fe_write returns 0 |
Date: |
Mon, 16 Jul 2018 11:03:30 +0200 |
On Fri, 13 Jul 2018 16:55:15 +0200
Marc-André Lureau <address@hidden> wrote:
> Hi
>
> On Tue, Jul 10, 2018 at 3:48 PM, Igor Mammedov <address@hidden> wrote:
> > On Tue, 5 Jun 2018 11:18:35 +0200
> > Paolo Bonzini <address@hidden> wrote:
> >
> >> On 05/06/2018 09:54, Sergio Lopez wrote:
> >> > Only retry on serial_xmit if qemu_chr_fe_write returns 0, as this is the
> >> > only recoverable error.
> >> >
> >> > Retrying with any other scenario, in addition to being a waste of CPU
> >> > cycles, can compromise the Guest stability if by the vCPU issuing the
> >> > write and the main loop thread are, by chance or explicit pinning,
> >> > running on the same pCPU.
> >> >
> >> > Previous discussion:
> >> >
> >> > https://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg06998.html
> >> >
> >> > Signed-off-by: Sergio Lopez <address@hidden>
> >> > ---
> >> > hw/char/serial.c | 2 +-
> >> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/hw/char/serial.c b/hw/char/serial.c
> >> > index 605b0d0..6de6c29 100644
> >> > --- a/hw/char/serial.c
> >> > +++ b/hw/char/serial.c
> >> > @@ -260,7 +260,7 @@ static void serial_xmit(SerialState *s)
> >> > 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 &&
> >> > + } else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) == 0 &&
> >> > s->tsr_retry < MAX_XMIT_RETRY) {
> >> > assert(s->watch_tag == 0);
> >> > s->watch_tag =
> >> >
> > Hi Sergio, Paolo,
> >
> > it looks like commit
> > commit 019288bf137183bf3407c9824655b753bfafc99f
> > Author: Sergio Lopez <address@hidden>
> > Date: Tue Jun 5 03:54:55 2018 -0400
> >
> > hw/char/serial: Only retry if qemu_chr_fe_write returns 0
> >
> > introduced regression wrt 2.12 and broke windows guest remote kernel debug
> > over
> > serial, where windbg can't connect to target and target hangs when windbg
> > tries
> > to connect to it.
> >
> > Reverting the commit fixes issue
> >
>
> is this a possible solution?
it fixes windbg over serial (with and without EINTR)
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index cd7d747c68..046c4684ff 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -261,15 +261,20 @@ static void serial_xmit(SerialState *s)
> 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) == 0 &&
> - s->tsr_retry < MAX_XMIT_RETRY) {
> - assert(s->watch_tag == 0);
> - s->watch_tag =
> - qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
> - serial_watch_cb, s);
> - if (s->watch_tag > 0) {
> - s->tsr_retry++;
> - return;
> + } else {
> + int rc = qemu_chr_fe_write(&s->chr, &s->tsr, 1);
> +
> + if ((rc == 0 ||
> + (rc == -1 && (errno == EAGAIN || errno == EINTR))) &&
> + s->tsr_retry < MAX_XMIT_RETRY) {
> + assert(s->watch_tag == 0);
> + s->watch_tag =
> + qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
> + serial_watch_cb, s);
> + if (s->watch_tag > 0) {
> + s->tsr_retry++;
> + return;
> + }
> }
> }
> s->tsr_retry = 0;
>
>