From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45355) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fOLMe-0001wt-FJ for qemu-devel@nongnu.org; Thu, 31 May 2018 07:03:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fOLMa-0000wr-Hf for qemu-devel@nongnu.org; Thu, 31 May 2018 07:03:28 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:52171) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fOLMa-0000vz-Bb for qemu-devel@nongnu.org; Thu, 31 May 2018 07:03:24 -0400 Received: by mail-wm0-f67.google.com with SMTP id r15-v6so25604209wmc.1 for ; Thu, 31 May 2018 04:03:24 -0700 (PDT) Date: Thu, 31 May 2018 13:03:21 +0200 From: Sergio Lopez Message-ID: <20180531110321.ltm3ti6onab2w3vg@dritchie> References: <20180531074601.10647-1-slp@redhat.com> <20180531074601.10647-4-slp@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 3/3] hw/char/serial: Don't retry on serial_xmit if errno == EPIPE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Cc: "Daniel P. Berrange" , Paolo Bonzini , "Michael S. Tsirkin" , QEMU On Thu, May 31, 2018 at 11:52:01AM +0200, Marc-André Lureau wrote: > On Thu, May 31, 2018 at 9:46 AM, Sergio Lopez wrote: > > If writing to the frontend channel failed with EPIPE, don't set up a > > retry. EPIPE is not a recoverable error, so trying again is a waste of CPU > > cycles. > > > > If the vCPU writing to the serial device and emulator thread are pinned > > to the same pCPU, it can also compromise the stability of the Guest OS, > > as both threads will be competing for pCPU's time, with the vCPU > > actively polling the serial device and barely giving time to the > > emulator thread to make actual progress. > > --- > > hw/char/serial.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/hw/char/serial.c b/hw/char/serial.c > > index 2c080c9..f26e86b 100644 > > --- a/hw/char/serial.c > > +++ b/hw/char/serial.c > > @@ -262,6 +262,7 @@ static void serial_xmit(SerialState *s) > > /* 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 && > > + errno != EPIPE && > > s->tsr_retry < MAX_XMIT_RETRY) { > > Instead of adding explicit handling of EPIPE, shouldn't the code be > rewritten to treat -1 return && errno != EAGAIN as fatal? What kind of action should it trigger if treating errno != EAGAIN as fatal? If you meant something like calling 'abort()', I think that, while this could be considered as the "correct" behavior, it's a bit unpractical for the users. The most common situation for this to happen is restarting virtlogd while having the console/serial redirected to it. The VM will stop writing to the serial device, but otherwise the Guest OS can probably keep working without any issues. Sergio.