From patchwork Thu Jun 2 17:17:21 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael S. Tsirkin" X-Patchwork-Id: 98447 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 1F176B6FAD for ; Fri, 3 Jun 2011 03:17:53 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753689Ab1FBRRe (ORCPT ); Thu, 2 Jun 2011 13:17:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19715 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753102Ab1FBRRc (ORCPT ); Thu, 2 Jun 2011 13:17:32 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p52HH6Yq000582 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 2 Jun 2011 13:17:06 -0400 Received: from redhat.com (reserved-201-247.tlv.redhat.com [10.35.201.247] (may be forged)) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with SMTP id p52HGxm8032726; Thu, 2 Jun 2011 13:17:00 -0400 Date: Thu, 2 Jun 2011 20:17:21 +0300 From: "Michael S. Tsirkin" To: linux-kernel@vger.kernel.org Cc: Rusty Russell , Carsten Otte , Christian Borntraeger , linux390@de.ibm.com, Martin Schwidefsky , Heiko Carstens , Shirley Ma , lguest@lists.ozlabs.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-s390@vger.kernel.org, kvm@vger.kernel.org, Krishna Kumar , Tom Lendacky , steved@us.ibm.com, habanero@linux.vnet.ibm.com Subject: Re: [PATCHv2 RFC 0/4] virtio and vhost-net capacity handling Message-ID: <20110602171721.GA13215@redhat.com> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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). --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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;