[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: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH] hw/char/serial: Only retry if qemu_chr_fe_write returns 0 |
Date: |
Fri, 13 Jul 2018 16:55:15 +0200 |
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?
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;
--
Marc-André Lureau