From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 7E2F62C50 for ; Mon, 7 Mar 2016 03:46:32 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP; 06 Mar 2016 18:46:31 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,549,1449561600"; d="scan'208";a="928209668" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by orsmga002.jf.intel.com with ESMTP; 06 Mar 2016 18:46:29 -0800 Date: Mon, 7 Mar 2016 10:48:38 +0800 From: Yuanhan Liu To: "Xie, Huawei" Message-ID: <20160307024838.GY14300@yliu-dev.sh.intel.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) 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:46:32 -0000 On Mon, Mar 07, 2016 at 02:32:46AM +0000, Xie, Huawei wrote: > On 3/4/2016 10:15 AM, Yuanhan Liu wrote: > > On Thu, Mar 03, 2016 at 04:30:42PM +0000, Xie, Huawei wrote: > >> On 2/18/2016 9:48 PM, Yuanhan Liu wrote: > >>> + mbuf_avail = 0; > >>> + mbuf_offset = 0; > >>> + while (desc_avail || (desc->flags & VRING_DESC_F_NEXT) != 0) { > >>> + /* This desc reachs to its end, get the next one */ > >>> + if (desc_avail == 0) { > >>> + desc = &vq->desc[desc->next]; > >>> + > >>> + desc_addr = gpa_to_vva(dev, desc->addr); > >>> + rte_prefetch0((void *)(uintptr_t)desc_addr); > >>> + > >>> + desc_offset = 0; > >>> + desc_avail = desc->len; > >>> + > >>> + PRINT_PACKET(dev, (uintptr_t)desc_addr, desc->len, 0); > >>> + } > >>> + > >>> + /* > >>> + * This mbuf reachs to its end, get a new one > >>> + * to hold more data. > >>> + */ > >>> + if (mbuf_avail == 0) { > >>> + cur = rte_pktmbuf_alloc(mbuf_pool); > >>> + if (unlikely(!cur)) { > >>> + RTE_LOG(ERR, VHOST_DATA, "Failed to " > >>> + "allocate memory for mbuf.\n"); > >>> + if (head) > >>> + rte_pktmbuf_free(head); > >>> + return NULL; > >>> + } > >> We could always allocate the head mbuf before the loop, then we save the > >> following branch and make the code more streamlined. > >> It reminds me that this change prevents the possibility of mbuf bulk > >> allocation, one solution is we pass the head mbuf from an additional > >> parameter. > > Yep, that's also something I have thought of. > > > >> Btw, put unlikely before the check of mbuf_avail and checks elsewhere. > > I don't think so. It would benifit for the small packets. What if, > > however, when TSO or jumbo frame is enabled that we have big packets? > > Prefer to favor the path that packet could fit in one mbuf. Hmmm, why? While I know that TSO and mergeable buf is disable by default in our vhost example vhost-switch, they are enabled by default in real life. > Btw, not specially to this, return "m = copy_desc_to_mbuf(dev, vq, > desc_indexes[i], mbuf_pool)", failure case is unlikely to happen, so add > unlikely for the check if (m == NULL) there. Please check all branches > elsewhere. Thanks for the remind, will have a detail check. --yliu