From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56783) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6JXZ-0000j6-An for qemu-devel@nongnu.org; Mon, 05 Aug 2013 08:05:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V6JXS-0002Nf-I5 for qemu-devel@nongnu.org; Mon, 05 Aug 2013 08:05:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19771) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6JXS-0002Ms-4O for qemu-devel@nongnu.org; Mon, 05 Aug 2013 08:05:26 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r75C5Pin024129 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 5 Aug 2013 08:05:25 -0400 Message-ID: <51FF956B.3060909@redhat.com> Date: Mon, 05 Aug 2013 15:07:07 +0300 From: Gal Hammer MIME-Version: 1.0 References: <1375690705-17266-1-git-send-email-ghammer@redhat.com> <51FF9130.3030201@redhat.com> In-Reply-To: <51FF9130.3030201@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio: Do not notify virtqueue if no element was pushed back. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: qemu-devel@nongnu.org On 05/08/2013 14:49, Laszlo Ersek wrote: > On 08/05/13 10:18, Gal Hammer wrote: >> Signed-off-by: Gal Hammer >> --- >> hw/char/virtio-serial-bus.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c >> index da417c7..0d38b4b 100644 >> --- a/hw/char/virtio-serial-bus.c >> +++ b/hw/char/virtio-serial-bus.c >> @@ -105,6 +105,7 @@ static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq, >> VirtIODevice *vdev) >> { >> VirtIOSerialPortClass *vsc; >> + bool elem_pushed = false; >> >> assert(port); >> assert(virtio_queue_ready(vq)); >> @@ -145,9 +146,12 @@ static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq, >> break; >> } >> virtqueue_push(vq, &port->elem, 0); >> + elem_pushed = true; >> port->elem.out_num = 0; >> } >> - virtio_notify(vdev, vq); >> + if (elem_pushed) { >> + virtio_notify(vdev, vq); >> + } >> } >> >> static void flush_queued_data(VirtIOSerialPort *port) >> > > I could be missing something, but it looks good to me. Thanks. > BTW the subject should say: "virtio-serial: ...", not generic "virtio: ...". OK. I can change that. > How did you catch this? Is this a performance problem, or did it expose > a guest driver bug? (The guest driver should be prepared for spurious > wakeups.) I had a bug in the Windows' driver that caused a buffer duplication when the char device didn't read the whole message. It was found when working with SPICE client over WAN (i.e. slow connection) and trying to copy&paste a large image. The driver was fixed. However I still think that this is needed although I'm not sure if there is a major performance penalty because of this redundant notification. Gal. > Reviewed-by: Laszlo Ersek >