From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757339Ab1E0WeW (ORCPT ); Fri, 27 May 2011 18:34:22 -0400 Received: from ozlabs.org ([203.10.76.45]:34419 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756806Ab1E0WeV (ORCPT ); Fri, 27 May 2011 18:34:21 -0400 From: Rusty Russell To: "Michael S. Tsirkin" 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 In-Reply-To: <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> <20110525060759.GC26352@redhat.com> User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.2.1 (i686-pc-linux-gnu) Date: Thu, 26 May 2011 12:58:23 +0930 Message-ID: <87vcwyjg2w.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 25 May 2011 09:07:59 +0300, "Michael S. Tsirkin" wrote: > On Wed, May 25, 2011 at 11:05:04AM +0930, Rusty Russell wrote: > 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. No, I started from the other direction: for (i = 0; i < 2; i++) { skb = get_buf(); if (!skb) break; kfree_skb(skb); } ie. free two packets for every one we're about to add. For steady state that would work really well. Then we hit the case where the ring seems full after we do the add: at that point, screw latency, and just try to free all the buffers we can. > 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? It makes sense, but I think it's a classic case where incremental improvements aren't as good as starting from scratch. Cheers, Rusty. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rusty Russell Subject: Re: [PATCHv2 10/14] virtio_net: limit xmit polling Date: Thu, 26 May 2011 12:58:23 +0930 Message-ID: <87vcwyjg2w.fsf@rustcorp.com.au> References: <877h9kvlps.fsf@rustcorp.com.au> <20110522121008.GA12155@redhat.com> <87boyutbjg.fsf@rustcorp.com.au> <20110523111900.GB27212@redhat.com> <8739k3k1fb.fsf@rustcorp.com.au> <20110525060759.GC26352@redhat.com> 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: "Michael S. Tsirkin" Return-path: In-Reply-To: <20110525060759.GC26352-H+wXaHxf7aLQT0dZR+AlfA@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, 25 May 2011 09:07:59 +0300, "Michael S. Tsirkin" wrote: > On Wed, May 25, 2011 at 11:05:04AM +0930, Rusty Russell wrote: > 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. No, I started from the other direction: for (i = 0; i < 2; i++) { skb = get_buf(); if (!skb) break; kfree_skb(skb); } ie. free two packets for every one we're about to add. For steady state that would work really well. Then we hit the case where the ring seems full after we do the add: at that point, screw latency, and just try to free all the buffers we can. > 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? It makes sense, but I think it's a classic case where incremental improvements aren't as good as starting from scratch. Cheers, Rusty.