From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net 2/3] virtio-net: fix num calculation on frag skb allocation failure Date: Wed, 20 Nov 2013 08:56:54 -0500 (EST) Message-ID: <1382353279.27479040.1384955814610.JavaMail.root@redhat.com> References: <1384938447-3775-1-git-send-email-jasowang@redhat.com> <1384938447-3775-2-git-send-email-jasowang@redhat.com> <20131120103729.GG19341@redhat.com> <734327384.27426189.1384949330819.JavaMail.root@redhat.com> <20131120133707.GA8523@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20131120133707.GA8523@redhat.com> 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: "Michael S. Tsirkin" Cc: Michael Dalton , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Eric Dumazet List-Id: virtualization@lists.linuxfoundation.org CgotLS0tLSDljp/lp4vpgq7ku7YgLS0tLS0KPiBPbiBXZWQsIE5vdiAyMCwgMjAxMyBhdCAwNzow ODo1MEFNIC0wNTAwLCBKYXNvbiBXYW5nIHdyb3RlOgo+ID4gCj4gPiAKPiA+IC0tLS0tIOWOn+Wn i+mCruS7tiAtLS0tLQo+ID4gPiBPbiBXZWQsIE5vdiAyMCwgMjAxMyBhdCAwNTowNzoyNlBNICsw ODAwLCBKYXNvbiBXYW5nIHdyb3RlOgo+ID4gPiA+IFdlIG5lZWQgZGVjcmVhc2UgdGhlIHJxLT5u dW0gYWZ0ZXIgd2UgY2FuIGdldCBhIGJ1ZiB0aHJvdWdoCj4gPiA+ID4gdmlydHF1ZXVlX2dldF9i dWYoKSBldmVuIGlmIHdlIGNvdWxkIG5vdCBhbGxvY2F0ZSBmcmFnIHNrYi4gT3RoZXJ3aXNlLAo+ ID4gPiA+IHRoZQo+ID4gPiA+IHJlZmlsbCByb3V0aW5lIHdvbid0IGJlIHRyaWdnZXJlZCB1bmRl ciBoZWF2eSBtZW1vcnkgc3RyZXNzIHNpbmNlIHRoZQo+ID4gPiA+IGRyaXZlciBtYXkKPiA+ID4g PiBzdGlsbCB0aGluayB0aGVyZSdzIGVub3VnaCByb29tLgo+ID4gPiA+IAo+ID4gPiA+IFRoaXMg YnVnIHdhcyBpbnRyb2R1Y2VkIGJ5IGNvbW1pdAo+ID4gPiA+IDI2MTNhZjBlZDE4YTExZDVjNTY2 YTgxZjlhNjUxMGI3MzE4MDY2MGEKPiA+ID4gPiAodmlydGlvX25ldDogbWlncmF0ZSBtZXJnZWFi bGUgcnggYnVmZmVycyB0byBwYWdlIGZyYWcgYWxsb2NhdG9ycykuCj4gPiA+ID4gCj4gPiA+ID4g Q2M6IFJ1c3R5IFJ1c3NlbGwgPHJ1c3R5QHJ1c3Rjb3JwLmNvbS5hdT4KPiA+ID4gPiBDYzogTWlj aGFlbCBTLiBUc2lya2luIDxtc3RAcmVkaGF0LmNvbT4KPiA+ID4gPiBDYzogTWljaGFlbCBEYWx0 b24gPG13ZGFsdG9uQGdvb2dsZS5jb20+Cj4gPiA+ID4gQ2M6IEVyaWMgRHVtYXpldCA8ZWR1bWF6 ZXRAZ29vZ2xlLmNvbT4KPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBKYXNvbiBXYW5nIDxqYXNvd2Fu Z0ByZWRoYXQuY29tPgo+ID4gPiAKPiA+ID4gU28gbGV0J3Mgd3JhcCB2aXJ0cXVldWVfZ2V0X2J1 ZiB0byBtYWtlIHN1cmUgd2UgZ2V0IGl0IHJpZ2h0Pwo+ID4gPiAKPiA+IAo+ID4gT2suIGdvb2Qg aWRlYS4KPiAKPiBTbyBJIGRpZCB0aGlzIChiZWxvdykgYnV0IHRoZSBjb21waWxlciBpcyBub3Qg c21hcnQgZW5vdWdoIHRvCj4gYXZvaWQgdHdvIGJyYW5jaGVzIG9uIGRhdGEgcGF0aC4KPiBTbyBJ J20gbm90IHN1cmUgYW55bW9yZTogd2l0aCBteSBwYXRjaCBpdCdzIHByZXR0eSBjbGVhciBob3cK PiB0aGUgY29kZSB3b3Jrcy4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBNaWNoYWVsIFMuIFRzaXJraW4g PG1zdEByZWRoYXQuY29tPgo+IAo+IGJ1dCBJIGRvbid0IHRoaW5rIHdlIG5lZWQgdG8gYXBwbHkg dGhpcy4KPiAKClRydWUsIGFub3RoZXIgcG9pbnQgaXMgd2UnZCBiZXR0ZXIgdG8gaGFuZGxlIGJv dGggaW5jcmVhc2luZyBhbmQgZGVjcmVhc2luZyBpbiB0aGUgCnNhbWUgd2F5LiBXZSBkbyBub3Qg aW5jcmVhc2UgaXQgaW4gYSBoZWxwZXIuCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0L3ZpcnRp b19uZXQuYyBiL2RyaXZlcnMvbmV0L3ZpcnRpb19uZXQuYwo+IGluZGV4IDExZDljYzkuLjFjYzJl NDMgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9uZXQvdmlydGlvX25ldC5jCj4gKysrIGIvZHJpdmVy cy9uZXQvdmlydGlvX25ldC5jCj4gQEAgLTI5Niw2ICsyOTYsMTQgQEAgc3RhdGljIHN0cnVjdCBz a19idWZmICpwYWdlX3RvX3NrYihzdHJ1Y3QgcmVjZWl2ZV9xdWV1ZQo+ICpycSwKPiAgCXJldHVy biBza2I7Cj4gIH0KPiAgCj4gK3N0YXRpYyB2b2lkICpycV9nZXRfYnVmKHN0cnVjdCByZWNlaXZl X3F1ZXVlICpycSwgdW5zaWduZWQgaW50ICpsZW4pCj4gK3sKPiArCXZvaWQgKmJ1ZiA9IHZpcnRx dWV1ZV9nZXRfYnVmKHJxLT52cSwgbGVuKTsKPiArCWlmIChidWYpCj4gKwkJcnEtPm51bS0tOwo+ ICsJcmV0dXJuIGJ1ZjsKPiArfQo+ICsKPiAgc3RhdGljIHN0cnVjdCBza19idWZmICpyZWNlaXZl X21lcmdlYWJsZShzdHJ1Y3QgbmV0X2RldmljZSAqZGV2LAo+ICAJCQkJCSBzdHJ1Y3QgcmVjZWl2 ZV9xdWV1ZSAqcnEsCj4gIAkJCQkJIHZvaWQgKmJ1ZiwKPiBAQCAtMzE1LDcgKzMyMyw3IEBAIHN0 YXRpYyBzdHJ1Y3Qgc2tfYnVmZiAqcmVjZWl2ZV9tZXJnZWFibGUoc3RydWN0Cj4gbmV0X2Rldmlj ZSAqZGV2LAo+ICAJd2hpbGUgKC0tbnVtX2J1Zikgewo+ICAJCWludCBudW1fc2tiX2ZyYWdzOwo+ ICAKPiAtCQlidWYgPSB2aXJ0cXVldWVfZ2V0X2J1ZihycS0+dnEsICZsZW4pOwo+ICsJCWJ1ZiA9 IHJxX2dldF9idWYocnEsICZsZW4pOwo+ICAJCWlmICh1bmxpa2VseSghYnVmKSkgewo+ICAJCQlw cl9kZWJ1ZygiJXM6IHJ4IGVycm9yOiAlZCBidWZmZXJzIG91dCBvZiAlZCBtaXNzaW5nXG4iLAo+ ICAJCQkJIGRldi0+bmFtZSwgbnVtX2J1ZiwgaGRyLT5taGRyLm51bV9idWZmZXJzKTsKPiBAQCAt MzI5LDcgKzMzNyw2IEBAIHN0YXRpYyBzdHJ1Y3Qgc2tfYnVmZiAqcmVjZWl2ZV9tZXJnZWFibGUo c3RydWN0Cj4gbmV0X2RldmljZSAqZGV2LAo+ICAJCX0KPiAgCj4gIAkJcGFnZSA9IHZpcnRfdG9f aGVhZF9wYWdlKGJ1Zik7Cj4gLQkJLS1ycS0+bnVtOwo+ICAKPiAgCQludW1fc2tiX2ZyYWdzID0g c2tiX3NoaW5mbyhjdXJyX3NrYiktPm5yX2ZyYWdzOwo+ICAJCWlmICh1bmxpa2VseShudW1fc2ti X2ZyYWdzID09IE1BWF9TS0JfRlJBR1MpKSB7Cj4gQEAgLTM3MCw3ICszNzcsNyBAQCBlcnJfYnVm Ogo+ICAJZGV2LT5zdGF0cy5yeF9kcm9wcGVkKys7Cj4gIAlkZXZfa2ZyZWVfc2tiKGhlYWRfc2ti KTsKPiAgCXdoaWxlICgtLW51bV9idWYpIHsKPiAtCQlidWYgPSB2aXJ0cXVldWVfZ2V0X2J1Zihy cS0+dnEsICZsZW4pOwo+ICsJCWJ1ZiA9IHJxX2dldF9idWYocnEsICZsZW4pOwo+ICAJCWlmICh1 bmxpa2VseSghYnVmKSkgewo+ICAJCQlwcl9kZWJ1ZygiJXM6IHJ4IGVycm9yOiAlZCBidWZmZXJz IG1pc3NpbmdcbiIsCj4gIAkJCQkgZGV2LT5uYW1lLCBudW1fYnVmKTsKPiBAQCAtMzc5LDcgKzM4 Niw2IEBAIGVycl9idWY6Cj4gIAkJfQo+ICAJCXBhZ2UgPSB2aXJ0X3RvX2hlYWRfcGFnZShidWYp Owo+ICAJCXB1dF9wYWdlKHBhZ2UpOwo+IC0JCS0tcnEtPm51bTsKPiAgCX0KPiAgCXJldHVybiBO VUxMOwo+ICB9Cj4gQEAgLTY3NSw5ICs2ODEsOCBAQCBzdGF0aWMgaW50IHZpcnRuZXRfcG9sbChz dHJ1Y3QgbmFwaV9zdHJ1Y3QgKm5hcGksIGludAo+IGJ1ZGdldCkKPiAgCj4gIGFnYWluOgo+ICAJ d2hpbGUgKHJlY2VpdmVkIDwgYnVkZ2V0ICYmCj4gLQkgICAgICAgKGJ1ZiA9IHZpcnRxdWV1ZV9n ZXRfYnVmKHJxLT52cSwgJmxlbikpICE9IE5VTEwpIHsKPiArCSAgICAgICAoYnVmID0gcnFfZ2V0 X2J1ZihycSwgJmxlbikpICE9IE5VTEwpIHsKPiAgCQlyZWNlaXZlX2J1ZihycSwgYnVmLCBsZW4p Owo+IC0JCS0tcnEtPm51bTsKPiAgCQlyZWNlaXZlZCsrOwo+ICAJfQo+ICAKPiAtLQo+IFRvIHVu c3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBuZXRk ZXYiIGluCj4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5v cmcKPiBNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9y ZG9tby1pbmZvLmh0bWwKPgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpWaXJ0dWFsaXphdGlvbiBtYWlsaW5nIGxpc3QKVmlydHVhbGl6YXRpb25AbGlzdHMu bGludXgtZm91bmRhdGlvbi5vcmcKaHR0cHM6Ly9saXN0cy5saW51eGZvdW5kYXRpb24ub3JnL21h aWxtYW4vbGlzdGluZm8vdmlydHVhbGl6YXRpb24= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753928Ab3KTN5E (ORCPT ); Wed, 20 Nov 2013 08:57:04 -0500 Received: from mx4-phx2.redhat.com ([209.132.183.25]:59130 "EHLO mx4-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751971Ab3KTN5C convert rfc822-to-8bit (ORCPT ); Wed, 20 Nov 2013 08:57:02 -0500 Date: Wed, 20 Nov 2013 08:56:54 -0500 (EST) From: Jason Wang To: "Michael S. Tsirkin" Cc: rusty@rustcorp.com.au, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Michael Dalton , Eric Dumazet Message-ID: <1382353279.27479040.1384955814610.JavaMail.root@redhat.com> In-Reply-To: <20131120133707.GA8523@redhat.com> References: <1384938447-3775-1-git-send-email-jasowang@redhat.com> <1384938447-3775-2-git-send-email-jasowang@redhat.com> <20131120103729.GG19341@redhat.com> <734327384.27426189.1384949330819.JavaMail.root@redhat.com> <20131120133707.GA8523@redhat.com> Subject: Re: [PATCH net 2/3] virtio-net: fix num calculation on frag skb allocation failure MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-Originating-IP: [10.5.82.11] X-Mailer: Zimbra 8.0.3_GA_5664 (ZimbraWebClient - GC31 (Win)/8.0.3_GA_5664) Thread-Topic: virtio-net: fix num calculation on frag skb allocation failure Thread-Index: cgqlu+a4lVAdjpkRe3SLlR25rxbCuw== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- 原始邮件 ----- > On Wed, Nov 20, 2013 at 07:08:50AM -0500, Jason Wang wrote: > > > > > > ----- 原始邮件 ----- > > > On Wed, Nov 20, 2013 at 05:07:26PM +0800, Jason Wang wrote: > > > > We need decrease the rq->num after we can get a buf through > > > > virtqueue_get_buf() even if we could not allocate frag skb. Otherwise, > > > > the > > > > refill routine won't be triggered under heavy memory stress since the > > > > driver may > > > > still think there's enough room. > > > > > > > > This bug was introduced by commit > > > > 2613af0ed18a11d5c566a81f9a6510b73180660a > > > > (virtio_net: migrate mergeable rx buffers to page frag allocators). > > > > > > > > Cc: Rusty Russell > > > > Cc: Michael S. Tsirkin > > > > Cc: Michael Dalton > > > > Cc: Eric Dumazet > > > > Signed-off-by: Jason Wang > > > > > > So let's wrap virtqueue_get_buf to make sure we get it right? > > > > > > > Ok. good idea. > > So I did this (below) but the compiler is not smart enough to > avoid two branches on data path. > So I'm not sure anymore: with my patch it's pretty clear how > the code works. > > Signed-off-by: Michael S. Tsirkin > > but I don't think we need to apply this. > True, another point is we'd better to handle both increasing and decreasing in the same way. We do not increase it in a helper. > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 11d9cc9..1cc2e43 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -296,6 +296,14 @@ static struct sk_buff *page_to_skb(struct receive_queue > *rq, > return skb; > } > > +static void *rq_get_buf(struct receive_queue *rq, unsigned int *len) > +{ > + void *buf = virtqueue_get_buf(rq->vq, len); > + if (buf) > + rq->num--; > + return buf; > +} > + > static struct sk_buff *receive_mergeable(struct net_device *dev, > struct receive_queue *rq, > void *buf, > @@ -315,7 +323,7 @@ static struct sk_buff *receive_mergeable(struct > net_device *dev, > while (--num_buf) { > int num_skb_frags; > > - buf = virtqueue_get_buf(rq->vq, &len); > + buf = rq_get_buf(rq, &len); > if (unlikely(!buf)) { > pr_debug("%s: rx error: %d buffers out of %d missing\n", > dev->name, num_buf, hdr->mhdr.num_buffers); > @@ -329,7 +337,6 @@ static struct sk_buff *receive_mergeable(struct > net_device *dev, > } > > page = virt_to_head_page(buf); > - --rq->num; > > num_skb_frags = skb_shinfo(curr_skb)->nr_frags; > if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) { > @@ -370,7 +377,7 @@ err_buf: > dev->stats.rx_dropped++; > dev_kfree_skb(head_skb); > while (--num_buf) { > - buf = virtqueue_get_buf(rq->vq, &len); > + buf = rq_get_buf(rq, &len); > if (unlikely(!buf)) { > pr_debug("%s: rx error: %d buffers missing\n", > dev->name, num_buf); > @@ -379,7 +386,6 @@ err_buf: > } > page = virt_to_head_page(buf); > put_page(page); > - --rq->num; > } > return NULL; > } > @@ -675,9 +681,8 @@ static int virtnet_poll(struct napi_struct *napi, int > budget) > > again: > while (received < budget && > - (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) { > + (buf = rq_get_buf(rq, &len)) != NULL) { > receive_buf(rq, buf, len); > - --rq->num; > received++; > } > > -- > 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 >