From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 3C8202C47 for ; Mon, 7 Mar 2016 03:33:40 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP; 06 Mar 2016 18:33:39 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,549,1449561600"; d="scan'208";a="759268859" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga003.jf.intel.com with ESMTP; 06 Mar 2016 18:33:38 -0800 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 6 Mar 2016 18:33:38 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 6 Mar 2016 18:33:38 -0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.136]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.18]) with mapi id 14.03.0248.002; Mon, 7 Mar 2016 10:32:46 +0800 From: "Xie, Huawei" To: Yuanhan Liu Thread-Topic: [PATCH v2 1/7] vhost: refactor rte_vhost_dequeue_burst Thread-Index: AdF1agd9clXhimYJTx2/yKoyh8rOiw== Date: Mon, 7 Mar 2016 02:32:46 +0000 Message-ID: References: <1449122773-25510-1-git-send-email-yuanhan.liu@linux.intel.com> <1455803352-5518-1-git-send-email-yuanhan.liu@linux.intel.com> <1455803352-5518-2-git-send-email-yuanhan.liu@linux.intel.com> <20160304021705.GT14300@yliu-dev.sh.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "Michael S. Tsirkin" , "dev@dpdk.org" , Victor Kaplansky Subject: Re: [dpdk-dev] [PATCH v2 1/7] vhost: refactor rte_vhost_dequeue_burst X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Mar 2016 02:33:41 -0000 On 3/4/2016 10:15 AM, Yuanhan Liu wrote:=0A= > On Thu, Mar 03, 2016 at 04:30:42PM +0000, Xie, Huawei wrote:=0A= >> On 2/18/2016 9:48 PM, Yuanhan Liu wrote:=0A= >>> + mbuf_avail =3D 0;=0A= >>> + mbuf_offset =3D 0;=0A= >>> + while (desc_avail || (desc->flags & VRING_DESC_F_NEXT) !=3D 0) {=0A= >>> + /* This desc reachs to its end, get the next one */=0A= >>> + if (desc_avail =3D=3D 0) {=0A= >>> + desc =3D &vq->desc[desc->next];=0A= >>> +=0A= >>> + desc_addr =3D gpa_to_vva(dev, desc->addr);=0A= >>> + rte_prefetch0((void *)(uintptr_t)desc_addr);=0A= >>> +=0A= >>> + desc_offset =3D 0;=0A= >>> + desc_avail =3D desc->len;=0A= >>> +=0A= >>> + PRINT_PACKET(dev, (uintptr_t)desc_addr, desc->len, 0);=0A= >>> + }=0A= >>> +=0A= >>> + /*=0A= >>> + * This mbuf reachs to its end, get a new one=0A= >>> + * to hold more data.=0A= >>> + */=0A= >>> + if (mbuf_avail =3D=3D 0) {=0A= >>> + cur =3D rte_pktmbuf_alloc(mbuf_pool);=0A= >>> + if (unlikely(!cur)) {=0A= >>> + RTE_LOG(ERR, VHOST_DATA, "Failed to "=0A= >>> + "allocate memory for mbuf.\n");=0A= >>> + if (head)=0A= >>> + rte_pktmbuf_free(head);=0A= >>> + return NULL;=0A= >>> + }=0A= >> We could always allocate the head mbuf before the loop, then we save the= =0A= >> following branch and make the code more streamlined.=0A= >> It reminds me that this change prevents the possibility of mbuf bulk=0A= >> allocation, one solution is we pass the head mbuf from an additional=0A= >> parameter.=0A= > Yep, that's also something I have thought of.=0A= >=0A= >> Btw, put unlikely before the check of mbuf_avail and checks elsewhere.= =0A= > I don't think so. It would benifit for the small packets. What if,=0A= > however, when TSO or jumbo frame is enabled that we have big packets?=0A= =0A= Prefer to favor the path that packet could fit in one mbuf.=0A= =0A= Btw, not specially to this, return "m =3D copy_desc_to_mbuf(dev, vq,=0A= desc_indexes[i], mbuf_pool)", failure case is unlikely to happen, so add=0A= unlikely for the check if (m =3D=3D NULL) there. Please check all branches= =0A= elsewhere.=0A= =0A= > --yliu=0A= >=0A= =0A=