From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44061) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VHesA-0004uz-Ub for qemu-devel@nongnu.org; Thu, 05 Sep 2013 15:05:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VHes3-0006fq-Bo for qemu-devel@nongnu.org; Thu, 05 Sep 2013 15:05:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:25841) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VHes3-0006fj-3P for qemu-devel@nongnu.org; Thu, 05 Sep 2013 15:05:35 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r85J5YbP007728 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 5 Sep 2013 15:05:34 -0400 Date: Fri, 6 Sep 2013 00:35:30 +0530 From: Amit Shah Message-ID: <20130905190530.GA23945@grmbl.mre> References: <1376227525-28639-1-git-send-email-ghammer@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1376227525-28639-1-git-send-email-ghammer@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2] virtio-serial: Do not notify virtqueue if no element was pushed back. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gal Hammer Cc: qemu-devel@nongnu.org Hi, On (Sun) 11 Aug 2013 [16:25:25], Gal Hammer wrote: > The redundant notification caused the Windows' driver to duplicate the > pending write request's buffer. The driver was fixed, but I think this > change is still required. > > 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); > + } > } Can you describe when you hit this bug in the commit message? I can imagine something like this happening when a fast guest is sending lots of data, while the host is slow to catch up, e.g. the host chardev is a file on disk, and that takes a while to flush the data from the guest, filling the chardev buffer, and causing the flow control to kick in. While in that state, if the guest writes more data to the virtqueue, a few iterations would occur where the host will not pop any item from the queue, and the guest is kicked unnecessarily. Is that what you're seeing? Your reply to v1 of the patch doesn't quite say that clearly. Patch looks good, thanks. Amit