add mailinglist scraper results

1 files changed, 834 insertions, 0 deletions

diff --git a/mailinglist/output_launchpad/1066055 b/mailinglist/output_launchpad/1066055
new file mode 100644
index 00000000..745abda2
--- /dev/null
+++ b/mailinglist/output_launchpad/1066055
@@ -0,0 +1,834 @@
+Network performance regression with vde_switch
+
+I've noticed a significant network performance regression when using vde_switch, starting about one week ago (10/05/2012); before that date, I used to get about 1.5 Gbits host to guest, but now I can only get about 320 Mbits; I didn't find any modification in net/vde.*, just in hw/virtio*.
+
+My command line: 
+ qemu-system-i386 -cdrom /bpd/bpd.iso -m 512 -boot d -enable-kvm \
+  -localtime -ctrl-grab -usbdevice tablet \
+  -device virtio-net-pci,mac=52:54:00:18:01:01,netdev=vde0,tx=bh,ioeventfd=on,x-txburst=32 \
+  -netdev vde,id=vde0 -vga std -tb-size 2M -cpu host -clock unix
+
+My host runs a kernel 3.6.1 and my guest runs a kernel 3.5.4; the same problem happens with other host and guest versions, too.
+
+I know there are better ways of running a guest, but using vde I get a cleaner environment in the host (just one tun/tap interface to manage...), which is quite good when running some accademic experiments.
+
+Interestingly, at the same time I've noticed a performance enhancement of about 25~30 % when using a tun/tap interface, bridged or not.
+
+Thank you, very much.
+
+Edivaldo de Araujo Pereira
+
+On Fri, Oct 12, 2012 at 05:34:23PM -0000, Edivaldo de Araujo Pereira wrote:
+> I've noticed a significant network performance regression when using
+> vde_switch, starting about one week ago (10/05/2012); before that date,
+> I used to get about 1.5 Gbits host to guest, but now I can only get
+> about 320 Mbits; I didn't find any modification in net/vde.*, just in
+> hw/virtio*.
+> 
+> My command line: 
+>  qemu-system-i386 -cdrom /bpd/bpd.iso -m 512 -boot d -enable-kvm \
+>   -localtime -ctrl-grab -usbdevice tablet \
+>   -device virtio-net-pci,mac=52:54:00:18:01:01,netdev=vde0,tx=bh,ioeventfd=on,x-txburst=32 \
+>   -netdev vde,id=vde0 -vga std -tb-size 2M -cpu host -clock unix
+> 
+> My host runs a kernel 3.6.1 and my guest runs a kernel 3.5.4; the same
+> problem happens with other host and guest versions, too.
+> 
+> I know there are better ways of running a guest, but using vde I get a
+> cleaner environment in the host (just one tun/tap interface to
+> manage...), which is quite good when running some accademic experiments.
+> 
+> Interestingly, at the same time I've noticed a performance enhancement
+> of about 25~30 % when using a tun/tap interface, bridged or not.
+
+Hi Edivaldo,
+It would be great if you can help find the commit that caused this
+regression.
+
+The basic process is:
+
+1. Identify a QEMU release or git tree that gives you 1.5 Gbit/s.
+2. Double-check that qemu.git/master suffers reduced performance.
+3. git bisect start <bad> <good>
+   where <bad> and <good> are the git commits that show differing
+   performance (for example, bad=HEAD good=v1.1.0)
+
+Then git will step through the commit history and ask you to test at
+each step.  (This is a binary search so even finding regressions that
+happened many commits ago requires few steps.)
+
+You can read more about git-bisect(1) here:
+http://git-scm.com/book/en/Git-Tools-Debugging-with-Git#Binary-Search
+http://www.kernel.org/pub/software/scm/git/docs/git-bisect.html
+
+The end result is the commit introduced the regression.  Please post
+what you find!
+
+Stefan
+
+
+Hi Stefan,

+

+Thank you, very much for taking the time to help me, and excuse me for not seeing your answer early... 

+

+I've run the procedure you pointed me out, and the result is:

+

+0d8d7690850eb0cf2b2b60933cf47669a6b6f18f is the first bad commit

+commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f

+Author: Amit Shah <email address hidden>

+Date:   Tue Sep 25 00:05:15 2012 +0530

+

+    virtio: Introduce virtqueue_get_avail_bytes()

+

+    The current virtqueue_avail_bytes() is oddly named, and checks if a

+    particular number of bytes are available in a vq.  A better API is to

+    fetch the number of bytes available in the vq, and let the caller do

+    what's interesting with the numbers.

+

+    Introduce virtqueue_get_avail_bytes(), which returns the number of bytes

+    for buffers marked for both, in as well as out.  virtqueue_avail_bytes()

+    is made a wrapper over this new function.

+

+    Signed-off-by: Amit Shah <email address hidden>

+    Signed-off-by: Michael S. Tsirkin <email address hidden>

+

+:040000 040000 1a58b06a228651cf844621d9ee2f49b525e36c93 e09ea66ce7f6874921670b6aeab5bea921a5227d M      hw

+

+I tried to revert that patch in the latest version, but it obviously didnt work; I'm trying to figure out the problem, but I don't know very well the souce code, so I think it's going to take some time. For now, it's all I could do.

+

+Thank you, again.

+Edivaldo

+

+
+
+On Mon, Oct 15, 2012 at 09:46:06PM -0000, Edivaldo de Araujo Pereira wrote:
+> Hi Stefan,
+> 
+> Thank you, very much for taking the time to help me, and excuse me for
+> not seeing your answer early...
+> 
+> I've run the procedure you pointed me out, and the result is:
+> 
+> 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f is the first bad commit
+> commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f
+> Author: Amit Shah <email address hidden>
+> Date:   Tue Sep 25 00:05:15 2012 +0530
+> 
+>     virtio: Introduce virtqueue_get_avail_bytes()
+> 
+>     The current virtqueue_avail_bytes() is oddly named, and checks if a
+>     particular number of bytes are available in a vq.  A better API is to
+>     fetch the number of bytes available in the vq, and let the caller do
+>     what's interesting with the numbers.
+> 
+>     Introduce virtqueue_get_avail_bytes(), which returns the number of bytes
+>     for buffers marked for both, in as well as out.  virtqueue_avail_bytes()
+>     is made a wrapper over this new function.
+> 
+>     Signed-off-by: Amit Shah <email address hidden>
+>     Signed-off-by: Michael S. Tsirkin <email address hidden>
+> 
+> :040000 040000 1a58b06a228651cf844621d9ee2f49b525e36c93
+> e09ea66ce7f6874921670b6aeab5bea921a5227d M      hw
+> 
+> I tried to revert that patch in the latest version, but it obviously
+> didnt work; I'm trying to figure out the problem, but I don't know very
+> well the souce code, so I think it's going to take some time. For now,
+> it's all I could do.
+
+After git-bisect(1) completes it is good to sanity-check the result by
+manually testing 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f^ (the commit
+just before the bad commit) and 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f
+(the bad commit).
+
+This will verify that the commit indeed introduces the regression.  I
+suggest doing this just to be sure that you've found the bad commit.
+
+Regarding this commit, I notice two things:
+
+1. We will now loop over all vring descriptors because we calculate the
+   total in/out length instead of returning early as soon as we see
+   there is enough space.  Maybe this makes a difference, although I'm a
+   little surprised you see such a huge regression.
+
+2. The comparision semantics have changed from:
+
+     (in_total += vring_desc_len(desc_pa, i)) >= in_bytes
+
+   to:
+
+     (in_bytes && in_bytes < in_total)
+
+   Notice that virtqueue_avail_bytes() now returns 0 when in_bytes ==
+   in_total.  Previously, it would return 1.  Perhaps we are starving or
+   delaying I/O due to this comparison change.  You can easily change
+   '<' to '<=' to see if it fixes the issue.
+
+Stefan
+
+
+Hi Stefan

+

+I finally could revert that commit in the latest snapshot; problem was I needed to revert one later, that modified hw/virtio-serial-bus.c accordingly; after that reversion, the regression in network performance went completely away; this confirms my previous identification of the commit that caused it.

+

+Additionally, I tested your last suggestion, to change '<' to '<=', and that didn't help; the problem was still there.

+

+By the way, the performance gain I observed ,of about 25~30 % when using a tun/tap, was in fact just apparent, because it was result of a greater use of cpu, so it was achieved only when the host was idle; when I stress the host, with a lot of concurrent guests generating traffic, there is no gain at all.

+

+Just for confirmation, this is the performance I get with latest snapshot (8b4a3df8081f3e6f1061ed5cbb303ad623ade66b) running wget in the guest:

+

+$ wget -O /dev/null http://172.18.1.1/bpd.iso

+--2012-10-16 09:10:18--  http://172.18.1.1/bpd.iso

+Connecting to 172.18.1.1:80... connected.

+HTTP request sent, awaiting response... 200 OK

+Length: 358979584 (342M) [application/x-iso9660-image]

+Saving to: `/dev/null'

+100%[======================================================>] 358.979.584 29,7M/s   in 11s

+2012-10-16 09:10:29 (30,3 MB/s) - `/dev/null' saved [358979584/358979584]

+

+The same wget, using the same snapshot, but with that commit reverted is:

+

+$ wget -O /dev/null http://172.18.1.1/bpd.iso

+--2012-10-16 09:15:12--  http://172.18.1.1/bpd.iso

+Connecting to 172.18.1.1:80... connected.

+HTTP request sent, awaiting response... 200 OK

+Length: 358979584 (342M) [application/x-iso9660-image]

+Saving to: `/dev/null'

+100%[======================================================>] 358.979.584  180M/s   in 1,9s

+2012-10-16 09:15:14 (180 MB/s) - `/dev/null' saved [358979584/358979584]

+

+So, as I can see, there is no doubt: that commit is the culprit; as it was intended to be related just to the quality of the source code (at least as I can see), but implied in such a cost in performance, I think it would be better to revert it.

+

+Thank you very much, again.

+Edivaldo
+
+On Tue, Oct 16, 2012 at 2:23 PM, Edivaldo de Araujo Pereira
+<email address hidden> wrote:
+> Hi Stefan
+>
+> I finally could revert that commit in the latest snapshot; problem was I
+> needed to revert one later, that modified hw/virtio-serial-bus.c
+> accordingly; after that reversion, the regression in network performance
+> went completely away; this confirms my previous identification of the
+> commit that caused it.
+>
+> Additionally, I tested your last suggestion, to change '<' to '<=', and
+> that didn't help; the problem was still there.
+>
+> By the way, the performance gain I observed ,of about 25~30 % when using
+> a tun/tap, was in fact just apparent, because it was result of a greater
+> use of cpu, so it was achieved only when the host was idle; when I
+> stress the host, with a lot of concurrent guests generating traffic,
+> there is no gain at all.
+>
+> Just for confirmation, this is the performance I get with latest
+> snapshot (8b4a3df8081f3e6f1061ed5cbb303ad623ade66b) running wget in the
+> guest:
+>
+> $ wget -O /dev/null http://172.18.1.1/bpd.iso
+> --2012-10-16 09:10:18--  http://172.18.1.1/bpd.iso
+> Connecting to 172.18.1.1:80... connected.
+> HTTP request sent, awaiting response... 200 OK
+> Length: 358979584 (342M) [application/x-iso9660-image]
+> Saving to: `/dev/null'
+> 100%[======================================================>] 358.979.584 29,7M/s   in 11s
+> 2012-10-16 09:10:29 (30,3 MB/s) - `/dev/null' saved [358979584/358979584]
+>
+> The same wget, using the same snapshot, but with that commit reverted
+> is:
+>
+> $ wget -O /dev/null http://172.18.1.1/bpd.iso
+> --2012-10-16 09:15:12--  http://172.18.1.1/bpd.iso
+> Connecting to 172.18.1.1:80... connected.
+> HTTP request sent, awaiting response... 200 OK
+> Length: 358979584 (342M) [application/x-iso9660-image]
+> Saving to: `/dev/null'
+> 100%[======================================================>] 358.979.584  180M/s   in 1,9s
+> 2012-10-16 09:15:14 (180 MB/s) - `/dev/null' saved [358979584/358979584]
+>
+> So, as I can see, there is no doubt: that commit is the culprit; as it
+> was intended to be related just to the quality of the source code (at
+> least as I can see), but implied in such a cost in performance, I think
+> it would be better to revert it.
+
+Hi Amit,
+Edivaldo has identified the following commit responsible for a network
+performance regression he sees:
+
+commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f
+Author: Amit Shah <email address hidden>
+Date:   Tue Sep 25 00:05:15 2012 +0530
+
+    virtio: Introduce virtqueue_get_avail_bytes()
+
+I guess this is because we now iterate the entire descriptor chain to
+check available space instead of returning early.
+
+Do you want to propose a patch to fix it?
+
+Stefan
+
+
+Dear Amit,
+
+On a suggestion of Stefan, I've already tested the modification in you patch, and it didn't work; but for confirmation I tested it once again, on the latest snapshot; same result, that is, it didn't work; the problem is still there.
+
+I didn't take enough time to uderstand the code, so unfortunately I fear there is not much I could do to solve the problem, apart from trying your suggestions. But I'll try to spend a little more time on it, until we find a solution.
+
+Thank you very much.
+
+Edivaldo
+
+--- Em seg, 22/10/12, Amit Shah <email address hidden> escreveu:
+
+> De: Amit Shah <email address hidden>
+> Assunto: Re: [Qemu-devel] [Bug 1066055] Re: Network performance regression with vde_switch
+> Para: "Stefan Hajnoczi" <email address hidden>
+> Cc: "Bug 1066055" <email address hidden>, <email address hidden>, <email address hidden>
+> Data: Segunda-feira, 22 de Outubro de 2012, 4:18
+> On (Tue) 16 Oct 2012 [09:48:09],
+> Stefan Hajnoczi wrote:
+> > On Mon, Oct 15, 2012 at 09:46:06PM -0000, Edivaldo de
+> Araujo Pereira wrote:
+> > > Hi Stefan,
+> > > 
+> > > Thank you, very much for taking the time to help
+> me, and excuse me for
+> > > not seeing your answer early...
+> > > 
+> > > I've run the procedure you pointed me out, and the
+> result is:
+> > > 
+> > > 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f is the
+> first bad commit
+> > > commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f
+> > > Author: Amit Shah <email address hidden>
+> > > Date:   Tue Sep 25 00:05:15 2012
+> +0530
+> > > 
+> > >     virtio: Introduce
+> virtqueue_get_avail_bytes()
+> > > 
+> > >     The current
+> virtqueue_avail_bytes() is oddly named, and checks if a
+> > >     particular number of bytes
+> are available in a vq.  A better API is to
+> > >     fetch the number of bytes
+> available in the vq, and let the caller do
+> > >     what's interesting with
+> the numbers.
+> > > 
+> > >     Introduce
+> virtqueue_get_avail_bytes(), which returns the number of
+> bytes
+> > >     for buffers marked for
+> both, in as well as out.  virtqueue_avail_bytes()
+> > >     is made a wrapper over
+> this new function.
+> > > 
+> > >     Signed-off-by: Amit Shah
+> <email address hidden>
+> > >     Signed-off-by: Michael S.
+> Tsirkin <email address hidden>
+> > > 
+> > > :040000 040000
+> 1a58b06a228651cf844621d9ee2f49b525e36c93
+> > > e09ea66ce7f6874921670b6aeab5bea921a5227d M 
+>     hw
+> > > 
+> > > I tried to revert that patch in the latest
+> version, but it obviously
+> > > didnt work; I'm trying to figure out the problem,
+> but I don't know very
+> > > well the souce code, so I think it's going to take
+> some time. For now,
+> > > it's all I could do.
+> > 
+> > After git-bisect(1) completes it is good to
+> sanity-check the result by
+> > manually testing
+> 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f^ (the commit
+> > just before the bad commit) and
+> 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f
+> > (the bad commit).
+> > 
+> > This will verify that the commit indeed introduces the
+> regression.  I
+> > suggest doing this just to be sure that you've found
+> the bad commit.
+> > 
+> > Regarding this commit, I notice two things:
+> > 
+> > 1. We will now loop over all vring descriptors because
+> we calculate the
+> >    total in/out length instead of returning
+> early as soon as we see
+> >    there is enough space.  Maybe this
+> makes a difference, although I'm a
+> >    little surprised you see such a huge
+> regression.
+> > 
+> > 2. The comparision semantics have changed from:
+> > 
+> >      (in_total +=
+> vring_desc_len(desc_pa, i)) >= in_bytes
+> > 
+> >    to:
+> > 
+> >      (in_bytes && in_bytes <
+> in_total)
+> > 
+> >    Notice that virtqueue_avail_bytes() now
+> returns 0 when in_bytes ==
+> >    in_total.  Previously, it would
+> return 1.  Perhaps we are starving or
+> >    delaying I/O due to this comparison
+> change.  You can easily change
+> >    '<' to '<=' to see if it fixes the
+> issue.
+> 
+> Hi Edivaldo,
+> 
+> Can you try the following patch, that will confirm if it's
+> the
+> descriptor walk or the botched compare that's causing the
+> regression.
+> 
+> Thanks,
+> 
+> diff --git a/hw/virtio.c b/hw/virtio.c
+> index 6821092..bb08ed8 100644
+> --- a/hw/virtio.c
+> +++ b/hw/virtio.c
+> @@ -406,8 +406,8 @@ int virtqueue_avail_bytes(VirtQueue *vq,
+> unsigned int in_bytes,
+>      unsigned int in_total, out_total;
+>  
+>      virtqueue_get_avail_bytes(vq,
+> &in_total, &out_total);
+> -    if ((in_bytes && in_bytes <
+> in_total)
+> -        || (out_bytes &&
+> out_bytes < out_total)) {
+> +    if ((in_bytes && in_bytes <=
+> in_total)
+> +        || (out_bytes &&
+> out_bytes <= out_total)) {
+>          return 1;
+>      }
+>      return 0;
+> 
+> 
+>         Amit
+> 
+
+
+On Mon, Oct 22, 2012 at 06:50:00AM -0700, Edivaldo de Araujo Pereira wrote:
+> I didn't take enough time to uderstand the code, so unfortunately I fear there is not much I could do to solve the problem, apart from trying your suggestions. But I'll try to spend a little more time on it, until we find a solution.
+
+I've thought a little about how to approach this.  Amit, here's a brain
+dump:
+
+The simplest solution is to make virtqueue_avail_bytes() use the old
+behavior of stopping early.
+
+However, I wonder if we can actually *improve* performance of existing
+code by changing virtio-net.c:virtio_net_receive().  The intuition is
+that calling virtio_net_has_buffers() (internally calls
+virtqueue_avail_bytes()) followed by virtqueue_pop() is suboptimal
+because we're repeatedly traversing the descriptor chain.
+
+We can get rid of this repetition.  A side-effect of this is that we no
+longer need to call virtqueue_avail_bytes() from virtio-net.c.  Here's
+how:
+
+The common case in virtio_net_receive() is that we have buffers and they
+are large enough for the received packet.  So to optimize for this case:
+
+1. Take the VirtQueueElement off the vring but don't increment
+   last_avail_idx yet.  (This is essentially a "peek" operation.)
+
+2. If there is an error or we drop the packet because the
+   VirtQueueElement is too small, just bail out and we'll grab the same
+   VirtQueueElement again next time.
+
+3. When we've committed filling in this VirtQueueElement, increment
+   last_avail_idx.  This is the point of no return.
+
+Essentially we're splitting pop() into peek() and consume().  Peek()
+grabs the VirtQueueElement but does not increment last_avail_idx.
+Consume() simply increments last_avail_idx and maybe the EVENT_IDX
+optimization stuff.
+
+Whether this will improve performance, I'm not sure.  Perhaps
+virtio_net_has_buffers() pulls most descriptors into the CPU's cache and
+following up with virtqueue_pop() is very cheap already.  But the idea
+here is to avoid the virtio_net_has_buffers() because we'll find out
+soon enough when we try to pop :).
+
+Another approach would be to drop virtio_net_has_buffers() but continue
+to use virtqueue_pop().  We'd keep the same VirtQueueElem stashed in
+VirtIONet across virtio_net_receive() calls in the case where we drop
+the packet.  I don't like this approach very much though because it gets
+tricky when the guest modifies the vring memory, resets the virtio
+device, etc across calls.
+
+Stefan
+
+
+On Thu, Nov 1, 2012 at 5:07 PM, Michael S. Tsirkin <email address hidden> wrote:
+> Commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f introduced
+> a regression in virtio-net performance because it looks
+> into the ring aggressively while we really only care
+> about a single packet worth of buffers.
+> To fix, add parameters limiting lookahead, and
+> use in virtqueue_avail_bytes.
+>
+> Signed-off-by: Michael S. Tsirkin <email address hidden>
+> Reported-by: Edivaldo de Araujo Pereira <email address hidden>
+
+Nice, much simpler than the ideas I had.
+
+Reviewed-by: Stefan Hajnoczi <email address hidden>
+
+
+On Fri, Nov 2, 2012 at 3:48 PM, Michael S. Tsirkin <email address hidden> wrote:
+> On Fri, Nov 02, 2012 at 11:18:18AM +0100, Stefan Hajnoczi wrote:
+>> On Thu, Nov 1, 2012 at 5:07 PM, Michael S. Tsirkin <email address hidden> wrote:
+>> > Commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f introduced
+>> > a regression in virtio-net performance because it looks
+>> > into the ring aggressively while we really only care
+>> > about a single packet worth of buffers.
+>> > To fix, add parameters limiting lookahead, and
+>> > use in virtqueue_avail_bytes.
+>> >
+>> > Signed-off-by: Michael S. Tsirkin <email address hidden>
+>> > Reported-by: Edivaldo de Araujo Pereira <email address hidden>
+>>
+>> Nice, much simpler than the ideas I had.
+>>
+>> Reviewed-by: Stefan Hajnoczi <email address hidden>
+>
+> Anthony could you apply this out of band please so this stops
+> biting people?
+
+Especially for the 1.3 release so that we don't have a virtio
+performance regression.
+
+Stefan
+
+
+Dear friends, 
+
+Please excuse-me for not reporting earlier... I confirm that the patch by Michael really fixes the problem I've reported. The regression has gone away when I used it, so I think it is good to be applied.
+
+Thanks,
+
+Edivaldo de Araújo Pereira
+
+
+--- Em ter, 27/11/12, Michael S. Tsirkin <email address hidden> escreveu:
+
+> De: Michael S. Tsirkin <email address hidden>
+> Assunto: Re: [PATCH] virtio: limit avail bytes lookahead
+> Para: "Amit Shah" <email address hidden>
+> Cc: "Stefan Hajnoczi" <email address hidden>, "Edivaldo de Araujo Pereira" <email address hidden>, <email address hidden>, "Anthony Liguori" <email address hidden>, "Bug 1066055" <email address hidden>
+> Data: Terça-feira, 27 de Novembro de 2012, 8:25
+> On Thu, Nov 01, 2012 at 06:07:21PM
+> +0200, Michael S. Tsirkin wrote:
+> > Commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f
+> introduced
+> > a regression in virtio-net performance because it
+> looks
+> > into the ring aggressively while we really only care
+> > about a single packet worth of buffers.
+> > To fix, add parameters limiting lookahead, and
+> > use in virtqueue_avail_bytes.
+> > 
+> > Signed-off-by: Michael S. Tsirkin <email address hidden>
+> > Reported-by: Edivaldo de Araujo Pereira <email address hidden>
+> 
+> Ping.
+> Anthony - going to apply this?
+> 
+> 
+> > ---
+> > 
+> > Edivaldo could you please confirm this fixes
+> regression?
+> > 
+> > diff --git a/hw/virtio-serial-bus.c
+> b/hw/virtio-serial-bus.c
+> > index d20bd8b..a761cdc 100644
+> > --- a/hw/virtio-serial-bus.c
+> > +++ b/hw/virtio-serial-bus.c
+> > @@ -297,7 +297,7 @@ size_t
+> virtio_serial_guest_ready(VirtIOSerialPort *port)
+> >      if (use_multiport(port->vser)
+> && !port->guest_connected) {
+> >          return 0;
+> >      }
+> > -    virtqueue_get_avail_bytes(vq,
+> &bytes, NULL);
+> > +    virtqueue_get_avail_bytes(vq,
+> &bytes, NULL, UINT_MAX, 0);
+> >      return bytes;
+> >  }
+> >  
+> > diff --git a/hw/virtio.c b/hw/virtio.c
+> > index ec8b7d8..f40a8c5 100644
+> > --- a/hw/virtio.c
+> > +++ b/hw/virtio.c
+> > @@ -336,7 +336,8 @@ static unsigned
+> virtqueue_next_desc(hwaddr desc_pa,
+> >  }
+> >  
+> >  void virtqueue_get_avail_bytes(VirtQueue *vq,
+> unsigned int *in_bytes,
+> > -             
+>              
+>    unsigned int *out_bytes)
+> > +             
+>              
+>    unsigned int *out_bytes,
+> > +             
+>              
+>    unsigned max_in_bytes, unsigned
+> max_out_bytes)
+> >  {
+> >      unsigned int idx;
+> >      unsigned int total_bufs, in_total,
+> out_total;
+> > @@ -385,6 +386,9 @@ void
+> virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int
+> *in_bytes,
+> >              } else
+> {
+> >               
+>   out_total += vring_desc_len(desc_pa, i);
+> >              }
+> > +            if (in_total
+> >= max_in_bytes && out_total >= max_out_bytes)
+> {
+> > +             
+>   goto done;
+> > +            }
+> >          } while ((i =
+> virtqueue_next_desc(desc_pa, i, max)) != max);
+> >  
+> >          if (!indirect)
+> > @@ -392,6 +396,7 @@ void
+> virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int
+> *in_bytes,
+> >          else
+> >             
+> total_bufs++;
+> >      }
+> > +done:
+> >      if (in_bytes) {
+> >          *in_bytes =
+> in_total;
+> >      }
+> > @@ -405,12 +410,8 @@ int
+> virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
+> >  {
+> >      unsigned int in_total, out_total;
+> >  
+> > -    virtqueue_get_avail_bytes(vq,
+> &in_total, &out_total);
+> > -    if ((in_bytes && in_bytes <
+> in_total)
+> > -        || (out_bytes &&
+> out_bytes < out_total)) {
+> > -        return 1;
+> > -    }
+> > -    return 0;
+> > +    virtqueue_get_avail_bytes(vq,
+> &in_total, &out_total, in_bytes, out_bytes);
+> > +    return in_bytes <= in_total
+> && out_bytes <= out_total;
+> >  }
+> >  
+> >  void virtqueue_map_sg(struct iovec *sg, hwaddr
+> *addr,
+> > diff --git a/hw/virtio.h b/hw/virtio.h
+> > index ac482be..1278328 100644
+> > --- a/hw/virtio.h
+> > +++ b/hw/virtio.h
+> > @@ -150,7 +150,8 @@ int virtqueue_pop(VirtQueue *vq,
+> VirtQueueElement *elem);
+> >  int virtqueue_avail_bytes(VirtQueue *vq, unsigned
+> int in_bytes,
+> >               
+>             unsigned int
+> out_bytes);
+> >  void virtqueue_get_avail_bytes(VirtQueue *vq,
+> unsigned int *in_bytes,
+> > -             
+>              
+>    unsigned int *out_bytes);
+> > +             
+>              
+>    unsigned int *out_bytes,
+> > +             
+>              
+>    unsigned max_in_bytes, unsigned
+> max_out_bytes);
+> >  
+> >  void virtio_notify(VirtIODevice *vdev, VirtQueue
+> *vq);
+> >  
+> 
+
+
+"Michael S. Tsirkin" <email address hidden> writes:
+
+> On Thu, Nov 01, 2012 at 06:07:21PM +0200, Michael S. Tsirkin wrote:
+>> Commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f introduced
+>> a regression in virtio-net performance because it looks
+>> into the ring aggressively while we really only care
+>> about a single packet worth of buffers.
+>> To fix, add parameters limiting lookahead, and
+>> use in virtqueue_avail_bytes.
+>> 
+>> Signed-off-by: Michael S. Tsirkin <email address hidden>
+>> Reported-by: Edivaldo de Araujo Pereira <email address hidden>
+>
+> Ping.
+> Anthony - going to apply this?
+
+Yes, I've got it queued now.  In the future, please top post patches.
+
+Regards,
+
+Anthony Liguori
+
+>
+>
+>> ---
+>> 
+>> Edivaldo could you please confirm this fixes regression?
+>> 
+>> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
+>> index d20bd8b..a761cdc 100644
+>> --- a/hw/virtio-serial-bus.c
+>> +++ b/hw/virtio-serial-bus.c
+>> @@ -297,7 +297,7 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
+>>      if (use_multiport(port->vser) && !port->guest_connected) {
+>>          return 0;
+>>      }
+>> -    virtqueue_get_avail_bytes(vq, &bytes, NULL);
+>> +    virtqueue_get_avail_bytes(vq, &bytes, NULL, UINT_MAX, 0);
+>>      return bytes;
+>>  }
+>>  
+>> diff --git a/hw/virtio.c b/hw/virtio.c
+>> index ec8b7d8..f40a8c5 100644
+>> --- a/hw/virtio.c
+>> +++ b/hw/virtio.c
+>> @@ -336,7 +336,8 @@ static unsigned virtqueue_next_desc(hwaddr desc_pa,
+>>  }
+>>  
+>>  void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
+>> -                               unsigned int *out_bytes)
+>> +                               unsigned int *out_bytes,
+>> +                               unsigned max_in_bytes, unsigned max_out_bytes)
+>>  {
+>>      unsigned int idx;
+>>      unsigned int total_bufs, in_total, out_total;
+>> @@ -385,6 +386,9 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
+>>              } else {
+>>                  out_total += vring_desc_len(desc_pa, i);
+>>              }
+>> +            if (in_total >= max_in_bytes && out_total >= max_out_bytes) {
+>> +                goto done;
+>> +            }
+>>          } while ((i = virtqueue_next_desc(desc_pa, i, max)) != max);
+>>  
+>>          if (!indirect)
+>> @@ -392,6 +396,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
+>>          else
+>>              total_bufs++;
+>>      }
+>> +done:
+>>      if (in_bytes) {
+>>          *in_bytes = in_total;
+>>      }
+>> @@ -405,12 +410,8 @@ int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
+>>  {
+>>      unsigned int in_total, out_total;
+>>  
+>> -    virtqueue_get_avail_bytes(vq, &in_total, &out_total);
+>> -    if ((in_bytes && in_bytes < in_total)
+>> -        || (out_bytes && out_bytes < out_total)) {
+>> -        return 1;
+>> -    }
+>> -    return 0;
+>> +    virtqueue_get_avail_bytes(vq, &in_total, &out_total, in_bytes, out_bytes);
+>> +    return in_bytes <= in_total && out_bytes <= out_total;
+>>  }
+>>  
+>>  void virtqueue_map_sg(struct iovec *sg, hwaddr *addr,
+>> diff --git a/hw/virtio.h b/hw/virtio.h
+>> index ac482be..1278328 100644
+>> --- a/hw/virtio.h
+>> +++ b/hw/virtio.h
+>> @@ -150,7 +150,8 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem);
+>>  int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
+>>                            unsigned int out_bytes);
+>>  void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
+>> -                               unsigned int *out_bytes);
+>> +                               unsigned int *out_bytes,
+>> +                               unsigned max_in_bytes, unsigned max_out_bytes);
+>>  
+>>  void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
+>>  
+
+
+"Michael S. Tsirkin" <email address hidden> writes:
+
+> On Thu, Nov 29, 2012 at 06:34:46PM +0530, Amit Shah wrote:
+>> On (Wed) 28 Nov 2012 [23:53:08], Michael S. Tsirkin wrote:
+>> > On Tue, Nov 27, 2012 at 06:25:04PM +0200, Michael S. Tsirkin wrote:
+>> > > On Thu, Nov 01, 2012 at 06:07:21PM +0200, Michael S. Tsirkin wrote:
+>> > > > Commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f introduced
+>> > > > a regression in virtio-net performance because it looks
+>> > > > into the ring aggressively while we really only care
+>> > > > about a single packet worth of buffers.
+>> > > > To fix, add parameters limiting lookahead, and
+>> > > > use in virtqueue_avail_bytes.
+>> > > > 
+>> > > > Signed-off-by: Michael S. Tsirkin <email address hidden>
+>> > > > Reported-by: Edivaldo de Araujo Pereira <email address hidden>
+>> > > 
+>> > > Ping.
+>> > > Anthony - going to apply this?
+>> > 
+>> > virtio rng was added since so naturally build broke.
+>> > Here's a patch on top to fix it up. I never used virtio rng before so
+>> > could not test at this hour, but it does fix the build.
+>> > 
+>> > I'll take a look at how to test it tomorrow but any
+>> > info would be appreciated.
+>> > Amit could you pls review?
+>> 
+>> Looks fine, I assume you will send a v2 of the patch to Anthony?
+>> 
+>> 		Amit
+>
+>
+> Anthony volunteered to test this so there will only be v2 if he sees
+> problems.
+
+I need a Signed-off-by so why don't you just go ahead and send a v2 and
+I'll test that.
+
+Regards,
+
+Anthony Liguori
+
+
+The fix had been included here:
+https://git.qemu.org/?p=qemu.git;a=commitdiff;h=e1f7b4812eab992de46c98b
+... so closing this bug now.
+