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 1FD242C56 for ; Mon, 30 May 2016 04:40:05 +0200 (CEST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP; 29 May 2016 19:40:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,387,1459839600"; d="scan'208";a="112610456" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga004.fm.intel.com with ESMTP; 29 May 2016 19:40:04 -0700 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, 29 May 2016 19:40:03 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 29 May 2016 19:40:02 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.150]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.181]) with mapi id 14.03.0248.002; Mon, 30 May 2016 10:40:01 +0800 From: "Xie, Huawei" To: Yuanhan Liu CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] virtio: split virtio rx/tx queue Thread-Index: AdG6HI+JBnSf06fSRheil2QVfceXwg== Date: Mon, 30 May 2016 02:40:00 +0000 Message-ID: References: <1462323027-91942-1-git-send-email-huawei.xie@intel.com> <1464097112-47411-1-git-send-email-huawei.xie@intel.com> <20160527090756.GC5641@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 Subject: Re: [dpdk-dev] [PATCH] virtio: split virtio rx/tx queue 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, 30 May 2016 02:40:06 -0000 On 5/27/2016 5:06 PM, Yuanhan Liu wrote:=0A= > On Tue, May 24, 2016 at 09:38:32PM +0800, Huawei Xie wrote:=0A= >> vq->vq_ring_mem =3D mz->phys_addr;=0A= >> vq->vq_ring_virt_mem =3D mz->addr;=0A= >> - PMD_INIT_LOG(DEBUG, "vq->vq_ring_mem: 0x%"PRIx64, (uint64_t)mz->p= hys_addr);=0A= >> - PMD_INIT_LOG(DEBUG, "vq->vq_ring_virt_mem: 0x%"PRIx64, (uint64_t)(uint= ptr_t)mz->addr);=0A= >> - vq->virtio_net_hdr_mz =3D NULL;=0A= >> - vq->virtio_net_hdr_mem =3D 0;=0A= >> + PMD_INIT_LOG(DEBUG, "vq->vq_ring_mem: 0x%"PRIx64,=0A= >> + (uint64_t)mz->phys_addr);=0A= >> + PMD_INIT_LOG(DEBUG, "vq->vq_ring_virt_mem: 0x%"PRIx64,=0A= >> + (uint64_t)(uintptr_t)mz->addr);=0A= >> +=0A= >> + hdr_mz =3D rte_memzone_reserve_aligned(vq_hdr_name, sz_hdr_mz, socket_= id,=0A= >> + 0, RTE_CACHE_LINE_SIZE);=0A= > We don't need allocate hdr_mz for Rx queue, and in such case, sz_hdr_mz= =0A= > is 0. I'm wondering what hdr_mz would be then, NULL?=0A= >=0A= > Anyway, you should skip the hdr_mz allocation for Rx queue, and I also=0A= > would suggest you to move the vq_hdr_name setup here.=0A= =0A= will check sz_hdr_mz before the zone allocation.=0A= =0A= =0A= >=0A= >> + if (hdr_mz =3D=3D NULL) {=0A= >> + if (rte_errno =3D=3D EEXIST)=0A= >> + hdr_mz =3D rte_memzone_lookup(vq_hdr_name);=0A= >> + if (hdr_mz =3D=3D NULL) {=0A= >> + ret =3D -ENOMEM;=0A= >> + goto fail_q_alloc;=0A= >> + }=0A= >> + }=0A= >> =0A= > ...=0A= >> =0A= >> PMD_INIT_FUNC_TRACE();=0A= >> ret =3D virtio_dev_queue_setup(dev, VTNET_CQ, VTNET_SQ_CQ_QUEUE_IDX,= =0A= >> - vtpci_queue_idx, 0, socket_id, &vq);=0A= >> + vtpci_queue_idx, 0, socket_id, (void **)&cvq);=0A= > Unnecessary cast. Note that there are few others like that in this=0A= > patch.=0A= =0A= This cast is needed.=0A= =0A= >=0A= >> - PMD_RX_LOG(DEBUG, "dequeue:%d", num);=0A= >> - PMD_RX_LOG(DEBUG, "packet len:%d", len[0]);=0A= >> + PMD_RX_LOG(DEBUG, "dequeue:%d\n", num);=0A= >> + PMD_RX_LOG(DEBUG, "packet len:%d\n", len[0]);=0A= > We should not append "\n" for PMD_RX_LOG; this macro alreadys does it.=0A= =0A= Weird. Will remove it. Thanks.=0A= =0A= >=0A= > Another note is that you might want to run checkpatch; I saw quite many= =0A= > warnings.=0A= =0A= Had checked. The warnings are all due to 80 char limitation of=0A= virtio_rxq_stats_strings. Just 4 or 5 chars cross 80 line limit. I=0A= prefer to keep the fields aligned.=0A= =0A= >=0A= > --yliu=0A= >=0A= =0A=