From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754486Ab1EYGIX (ORCPT ); Wed, 25 May 2011 02:08:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27882 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752324Ab1EYGIV (ORCPT ); Wed, 25 May 2011 02:08:21 -0400 Date: Wed, 25 May 2011 09:07:59 +0300 From: "Michael S. Tsirkin" To: Rusty Russell Cc: linux-kernel@vger.kernel.org, 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 10/14] virtio_net: limit xmit polling Message-ID: <20110525060759.GC26352@redhat.com> References: <877h9kvlps.fsf@rustcorp.com.au> <20110522121008.GA12155@redhat.com> <87boyutbjg.fsf@rustcorp.com.au> <20110523111900.GB27212@redhat.com> <8739k3k1fb.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8739k3k1fb.fsf@rustcorp.com.au> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, May 25, 2011 at 11:05:04AM +0930, Rusty Russell wrote: > On Mon, 23 May 2011 14:19:00 +0300, "Michael S. Tsirkin" wrote: > > I do understand how it seems a waste to leave direct space > > in the ring while we might in practice have space > > due to indirect. Didn't come up with a nice way to > > solve this yet - but 'no worse than now :)' > > Let's just make it "bool free_old_xmit_skbs(unsigned int max)". max == > 2 for the normal xmit path, so we're low latency but we keep ahead on > average. max == -1 for the "we're out of capacity, we may have to stop > the queue". > > That keeps it simple and probably the right thing... > > Thanks, > Rusty. Hmm I'm not sure I got it, need to think about this. I'd like to go back and document how my design was supposed to work. This really should have been in commit log or even a comment. I thought we need a min, not a max. We start with this: while ((c = (virtqueue_get_capacity(vq) < 2 + MAX_SKB_FRAGS) && (skb = get_buf))) kfree_skb(skb); return !c; This is clean and simple, right? And it's exactly asking for what we need. But this way we always keep a lot of memory in skbs even when rate of communication is low. So we add the min parameter: int n = 0; while ((((c = (virtqueue_get_capacity(vq) < 2 + MAX_SKB_FRAGS)) || n++ < min) && (skb = get_buf))) kfree_skb(skb); return !c; on the normal path min == 2 so we're low latency but we keep ahead on average. min == 0 for the "we're out of capacity, we may have to stop the queue". Does the above make sense at all? -- MST From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCHv2 10/14] virtio_net: limit xmit polling Date: Wed, 25 May 2011 09:07:59 +0300 Message-ID: <20110525060759.GC26352@redhat.com> References: <877h9kvlps.fsf@rustcorp.com.au> <20110522121008.GA12155@redhat.com> <87boyutbjg.fsf@rustcorp.com.au> <20110523111900.GB27212@redhat.com> <8739k3k1fb.fsf@rustcorp.com.au> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: Krishna Kumar , Carsten Otte , lguest-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, Shirley Ma , kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org, Heiko Carstens , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, steved-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org, Christian Borntraeger , Tom Lendacky , Martin Schwidefsky , linux390-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org To: Rusty Russell Return-path: Content-Disposition: inline In-Reply-To: <8739k3k1fb.fsf-8n+1lVoiYb80n/F98K4Iww@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: lguest-bounces+glkvl-lguest=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: lguest-bounces+glkvl-lguest=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org List-Id: netdev.vger.kernel.org On Wed, May 25, 2011 at 11:05:04AM +0930, Rusty Russell wrote: > On Mon, 23 May 2011 14:19:00 +0300, "Michael S. Tsirkin" wrote: > > I do understand how it seems a waste to leave direct space > > in the ring while we might in practice have space > > due to indirect. Didn't come up with a nice way to > > solve this yet - but 'no worse than now :)' > > Let's just make it "bool free_old_xmit_skbs(unsigned int max)". max == > 2 for the normal xmit path, so we're low latency but we keep ahead on > average. max == -1 for the "we're out of capacity, we may have to stop > the queue". > > That keeps it simple and probably the right thing... > > Thanks, > Rusty. Hmm I'm not sure I got it, need to think about this. I'd like to go back and document how my design was supposed to work. This really should have been in commit log or even a comment. I thought we need a min, not a max. We start with this: while ((c = (virtqueue_get_capacity(vq) < 2 + MAX_SKB_FRAGS) && (skb = get_buf))) kfree_skb(skb); return !c; This is clean and simple, right? And it's exactly asking for what we need. But this way we always keep a lot of memory in skbs even when rate of communication is low. So we add the min parameter: int n = 0; while ((((c = (virtqueue_get_capacity(vq) < 2 + MAX_SKB_FRAGS)) || n++ < min) && (skb = get_buf))) kfree_skb(skb); return !c; on the normal path min == 2 so we're low latency but we keep ahead on average. min == 0 for the "we're out of capacity, we may have to stop the queue". Does the above make sense at all? -- MST