Re: [Qemu-devel] [PATCH v2] virtio-serial: Do not notify virtqueue if no element was pushed back.

[Amit Shah <amit.shah@redhat.com>]

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 <ghammer@redhat.com>
> ---
>  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