From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rusty Russell Subject: Re: [RFC PATCH v1 2/2] virtio_net: Don't disable napi on low memory. Date: Mon, 09 Jan 2012 17:16:52 +1030 Message-ID: <877h119xur.fsf@rustcorp.com.au> References: <20120104225223.18184.1537.stgit@mike2.sea.corp.google.com> <20120104225237.18184.71853.stgit@mike2.sea.corp.google.com> <87y5tnat2g.fsf@rustcorp.com.au> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Mike Waychison Cc: netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, "Michael S. Tsirkin" List-Id: virtualization@lists.linuxfoundation.org T24gRnJpLCA2IEphbiAyMDEyIDA5OjU0OjQ2IC0wODAwLCBNaWtlIFdheWNoaXNvbiA8bWlrZXdA Z29vZ2xlLmNvbT4gd3JvdGU6Cj4gT24gV2VkLCBKYW4gNCwgMjAxMiBhdCA2OjQ2IFBNLCBNaWtl IFdheWNoaXNvbiA8bWlrZXdAZ29vZ2xlLmNvbT4gd3JvdGU6Cj4gPiBPbiBXZWQsIEphbiA0LCAy MDEyIGF0IDQ6MzEgUE0sIFJ1c3R5IFJ1c3NlbGwgPHJ1c3R5QHJ1c3Rjb3JwLmNvbS5hdT4gd3Jv dGU6Cj4gPj4gNCkgWW91IHVzZSB0aGUgc2tiIGRhdGEgZm9yIHRoZSBsaW5rZWQgbGlzdDsgdXNl IHRoZSBza2IgaGVhZCdzIGxpc3QuCj4gCj4gV2hhdCBkaWQgeW91IG1lYW4gYnkgdGhpcz8gIEkg d2FzIHVuZGVyIHRoZSBpbXByZXNzaW9uIHRoYXQgdGhlIC0+bmV4dAo+IGFuZCAtPnByZXYgZmll bGRzIGluIHNrX2J1ZmYgd2VyZSB0aGUgZmlyc3QgdHdvIGVsZW1lbnRzIHNwZWNpZmljYWxseQo+ IHNvIHRoYXQgdGhlIHBvaW50ZXIgY291bGQgYmUgdHJlYXRlZCBhcyBhIGxpc3RfaGVhZC4gIElm IGl0J3MgdGhlIGNhc3QKPiBpbiBwYXJ0aWN1bGFyIHRoYXQgeW91IGhhdmUgYW4gb2JqZWN0aW9u IHdpdGgsIEkgY2FuIGVhc2lseSBjaGFuZ2UKPiB0aGlzIHRvIGEgc2luZ2x5IGxpbmtlZCBsaXN0 IHRocmVhZGVkIHRocm91Z2ggLT5uZXh0IGlmIHRoYXQncwo+IGNsZWFuZXIuCgpZZXAsIEkgc2F3 IHRoZSBjYXN0IGFuZCBtaXNyZWFkIHlvdXIgY29kZS4gIEkgY291bGQgaGF2ZSBzd29ybiB0aGF0 IHNrYgp1c2VkIGEgcmVhbCBsaXN0X2hlYWQgdGhlc2UgZGF5cywgYnV0IEknbSB3cm9uZy4KCj4g Pj4KPiA+PiBJbnN0ZWFkLCBoZXJlJ3MgaG93IEkgdGhpbmsgaXQgc2hvdWxkIGJlIGRvbmU6Cj4g Li4uCj4gPgo+ID4gVGhpcyBzb3VuZHMgcmVhc29uYWJsZSB0byBtZS4gwqBJJ2xsIHNlZSB3aGF0 IEkgY2FuIG11c3RlciB0b2dldGhlciB0aGlzIHdlZWsuCj4gPgo+IAo+IFNvIEkgc3RhcnRlZCBp bXBsZW1lbnRpbmcgaXQgdGhlIHdheSB5b3Ugd2VyZSBtZW50aW9uaW5nLCBhbmQgcmFuIGludG8K PiBhIHByb2JsZW0gd2l0aCB0aGUgb3JpZ2luYWwgcGF0Y2hzZXQuCj4gCj4gQ3VycmVudGx5IHRo ZSAibWVyZ2VhYmxlIiBhbmQgImJpZyIgcmVjZWl2ZSBidWZmZXJzIHVzZSBhIHByaXZhdGUgcGFn ZQo+IGZyZWUgbGlzdCAodmlydG5ldF9pbmZvLT5wYWdlcykgd2hpY2ggaGFzIG5vIHN5bmNocm9u aXphdGlvbiBpdHNlbGYuCj4gVGhpcyBtZWFucyB0aGF0IHRoZSBiYXRjaGVkIHZlcnNpb24gY2Fu J3QgdXNlIGdldF9hX3BhZ2UoKSBhbmQKPiBnaXZlX3BhZ2VzKCkgYXMgaXMsIHdoaWNoIHJlZHVj ZXMgdGhlIG5lZWQgdG8gcmUtdXNlIHRoZSBzYW1lIGFsbG9jCj4gaGFsdmVzIHRoYXQgSSd2ZSBz cGxpdC4gICBBbHRlcm5hdGl2ZXMgSSBjYW4gdGhpbmsgb2YgYXQgdGhpcyBwb2ludDoKPiAKPiAt IHBhc3MgaW4gYSBmbGFnIHRvIHRoZSBhbGxvY2F0b3JzIGxpa2UgImJvb2wgaXNfc2VyaWFsIiB0 aGF0IGlzIHRydWUKPiBpZiB3ZSBhcmUgc2VyaWFsaXppbmcgd2l0aCBuYXBpLCAod2hpY2ggZGV0 ZXJtaW5lcyBpZiB3ZSBjYW4gbXVjaCB3aXRoCj4gdmktPnBhZ2VzKQo+IG9yCj4gLSBub3QgdXNl IHRoZSBzYW1lIGFsbG9jYXRvcnMgZm9yIHRoZSAibWVyZ2VhYmxlIiBhbmQgImJpZyIgcGF0aHMu Cj4gVGhlIG1lcmdlYWJsZSBhbGxvY2F0b3IgaW4gdGhlIG5vbi1zZXJpYWxpemVkIGNhc2UgcmVk dWNlcyB0bwo+IGFsbG9jX3BhZ2UoKSwgd2hpbGUgdGhlIGJpZyBhbGxvY2F0b3IgbG9va3MgbGlr ZSBhIGNvcHkgYW5kIHBhc3RlIHRoYXQKPiB1c2VzIGFsbG9jX3BhZ2UgaW5zdGVhZCBvZiBnZXRf YV9wYWdlKCkuCj4gCj4gUHJlZmVyZW5jZXM/ICBJJ2xsIGNvZGUgb25lIG9mIHRoZSB0d28gdXAg YW5kIHNlZSB3aGF0IGl0IGxvb2tzIGxpa2UuCgpXaGF0ZXZlciByZXN1bHRzIGluIGEgY2xlYW5l ciBkcml2ZXIsIEknbSBoYXBweS4KClRoYW5rcywKUnVzdHkuCl9fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fClZpcnR1YWxpemF0aW9uIG1haWxpbmcgbGlzdApW aXJ0dWFsaXphdGlvbkBsaXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZwpodHRwczovL2xpc3RzLmxp bnV4Zm91bmRhdGlvbi5vcmcvbWFpbG1hbi9saXN0aW5mby92aXJ0dWFsaXphdGlvbg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755411Ab2AIHMh (ORCPT ); Mon, 9 Jan 2012 02:12:37 -0500 Received: from ozlabs.org ([203.10.76.45]:53632 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754296Ab2AIHMf convert rfc822-to-8bit (ORCPT ); Mon, 9 Jan 2012 02:12:35 -0500 From: Rusty Russell To: Mike Waychison Cc: "Michael S. Tsirkin" , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org Subject: Re: [RFC PATCH v1 2/2] virtio_net: Don't disable napi on low memory. In-Reply-To: References: <20120104225223.18184.1537.stgit@mike2.sea.corp.google.com> <20120104225237.18184.71853.stgit@mike2.sea.corp.google.com> <87y5tnat2g.fsf@rustcorp.com.au> User-Agent: Notmuch/0.6.1-1 (http://notmuchmail.org) Emacs/23.3.1 (i686-pc-linux-gnu) Date: Mon, 09 Jan 2012 17:16:52 +1030 Message-ID: <877h119xur.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 6 Jan 2012 09:54:46 -0800, Mike Waychison wrote: > On Wed, Jan 4, 2012 at 6:46 PM, Mike Waychison wrote: > > On Wed, Jan 4, 2012 at 4:31 PM, Rusty Russell wrote: > >> 4) You use the skb data for the linked list; use the skb head's list. > > What did you mean by this? I was under the impression that the ->next > and ->prev fields in sk_buff were the first two elements specifically > so that the pointer could be treated as a list_head. If it's the cast > in particular that you have an objection with, I can easily change > this to a singly linked list threaded through ->next if that's > cleaner. Yep, I saw the cast and misread your code. I could have sworn that skb used a real list_head these days, but I'm wrong. > >> > >> Instead, here's how I think it should be done: > ... > > > > This sounds reasonable to me.  I'll see what I can muster together this week. > > > > So I started implementing it the way you were mentioning, and ran into > a problem with the original patchset. > > Currently the "mergeable" and "big" receive buffers use a private page > free list (virtnet_info->pages) which has no synchronization itself. > This means that the batched version can't use get_a_page() and > give_pages() as is, which reduces the need to re-use the same alloc > halves that I've split. Alternatives I can think of at this point: > > - pass in a flag to the allocators like "bool is_serial" that is true > if we are serializing with napi, (which determines if we can much with > vi->pages) > or > - not use the same allocators for the "mergeable" and "big" paths. > The mergeable allocator in the non-serialized case reduces to > alloc_page(), while the big allocator looks like a copy and paste that > uses alloc_page instead of get_a_page(). > > Preferences? I'll code one of the two up and see what it looks like. Whatever results in a cleaner driver, I'm happy. Thanks, Rusty.