[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110602171721.GA13215@redhat.com>
Date: Thu, 2 Jun 2011 20:17:21 +0300
From: "Michael S. Tsirkin" <mst@...hat.com>
To: linux-kernel@...r.kernel.org
Cc: Rusty Russell <rusty@...tcorp.com.au>,
Carsten Otte <cotte@...ibm.com>,
Christian Borntraeger <borntraeger@...ibm.com>,
linux390@...ibm.com, Martin Schwidefsky <schwidefsky@...ibm.com>,
Heiko Carstens <heiko.carstens@...ibm.com>,
Shirley Ma <xma@...ibm.com>, lguest@...ts.ozlabs.org,
virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org,
linux-s390@...r.kernel.org, kvm@...r.kernel.org,
Krishna Kumar <krkumar2@...ibm.com>,
Tom Lendacky <tahm@...ux.vnet.ibm.com>, steved@...ibm.com,
habanero@...ux.vnet.ibm.com
Subject: Re: [PATCHv2 RFC 0/4] virtio and vhost-net capacity handling
On Thu, Jun 02, 2011 at 06:42:35PM +0300, Michael S. Tsirkin wrote:
> OK, here's a new attempt to use the new capacity api. I also added more
> comments to clarify the logic. Hope this is more readable. Let me know
> pls.
>
> This is on top of the patches applied by Rusty.
>
> Warning: untested. Posting now to give people chance to
> comment on the API.
>
> Changes from v1:
> - fix comment in patch 2 to correct confusion noted by Rusty
> - rewrite patch 3 along the lines suggested by Rusty
> note: it's not exactly the same but I hope it's close
> enough, the main difference is that mine does limited
> polling even in the unlikely xmit failure case.
> - added a patch to not return capacity from add_buf
> it always looked like a weird hack
>
> Michael S. Tsirkin (4):
> virtio_ring: add capacity check API
> virtio_net: fix tx capacity checks using new API
> virtio_net: limit xmit polling
> Revert "virtio: make add_buf return capacity remaining:
>
> drivers/net/virtio_net.c | 111 ++++++++++++++++++++++++++----------------
> drivers/virtio/virtio_ring.c | 10 +++-
> include/linux/virtio.h | 7 ++-
> 3 files changed, 84 insertions(+), 44 deletions(-)
>
> --
> 1.7.5.53.gc233e
And just FYI, here's a patch (on top) that I considered but
decided against. This does a single get_buf before
xmit. I thought it's not really needed as the capacity
check in add_buf is relatively cheap, and we removed
the kick in xmit_skb. But the point is that the loop
will now be easy to move around if someone manages
to show this benefits speed (which I doubt).
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b25db1c..75af5be 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -590,6 +590,9 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
struct virtnet_info *vi = netdev_priv(dev);
int ret, i;
+ /* Try to pop an skb, to increase the chance we can add this one. */
+ free_old_xmit_skb(v);
+
/* We normally do have space in the ring, so try to queue the skb as
* fast as possible. */
ret = xmit_skb(vi, skb);
@@ -627,8 +630,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
* This is so that we don't hog the skb memory unnecessarily. *
* Doing this after kick means there's a chance we'll free
* the skb we have just sent, which is hot in cache. */
- for (i = 0; i < 2; i++)
- free_old_xmit_skb(v);
+ free_old_xmit_skb(v);
if (likely(free_xmit_capacity(vi)))
return NETDEV_TX_OK;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists