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 90281594B for ; Mon, 21 Dec 2015 16:21:44 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga102.jf.intel.com with ESMTP; 21 Dec 2015 07:21:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,460,1444719600"; d="scan'208";a="878277028" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga002.fm.intel.com with ESMTP; 21 Dec 2015 07:21:40 -0800 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 21 Dec 2015 07:21:40 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 21 Dec 2015 07:21:40 -0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.190]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.28]) with mapi id 14.03.0248.002; Mon, 21 Dec 2015 23:21:37 +0800 From: "Xie, Huawei" To: "Wiles, Keith" , Stephen Hemminger , "Ananyev, Konstantin" Thread-Topic: [dpdk-dev] [PATCH v2 1/2] mbuf: provide rte_pktmbuf_alloc_bulk API Thread-Index: AdE8A0gB5k4AFZq3TfmHtb59BK82Zg== Date: Mon, 21 Dec 2015 15:21:36 +0000 Message-ID: References: <1450049754-33635-1-git-send-email-huawei.xie@intel.com> <1450055682-51953-1-git-send-email-huawei.xie@intel.com> <1450055682-51953-2-git-send-email-huawei.xie@intel.com> <20151217210114.534a7561@xeon-e3> <2601191342CEEE43887BDE71AB97725836AD5AEB@irsmsx105.ger.corp.intel.com> <20151218093206.35e2e3e4@xeon-e3> <8987AF45-5666-42AB-8358-09025143205B@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: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 1/2] mbuf: provide rte_pktmbuf_alloc_bulk API 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, 21 Dec 2015 15:21:45 -0000 On 12/19/2015 3:27 AM, Wiles, Keith wrote:=0A= > On 12/18/15, 11:32 AM, "dev on behalf of Stephen Hemminger" wrote:=0A= >=0A= >> On Fri, 18 Dec 2015 10:44:02 +0000=0A= >> "Ananyev, Konstantin" wrote:=0A= >>=0A= >>>=0A= >>>> -----Original Message-----=0A= >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger= =0A= >>>> Sent: Friday, December 18, 2015 5:01 AM=0A= >>>> To: Xie, Huawei=0A= >>>> Cc: dev@dpdk.org=0A= >>>> Subject: Re: [dpdk-dev] [PATCH v2 1/2] mbuf: provide rte_pktmbuf_alloc= _bulk API=0A= >>>>=0A= >>>> On Mon, 14 Dec 2015 09:14:41 +0800=0A= >>>> Huawei Xie wrote:=0A= >>>>=0A= >>>>> v2 changes:=0A= >>>>> unroll the loop a bit to help the performance=0A= >>>>>=0A= >>>>> rte_pktmbuf_alloc_bulk allocates a bulk of packet mbufs.=0A= >>>>>=0A= >>>>> There is related thread about this bulk API.=0A= >>>>> http://dpdk.org/dev/patchwork/patch/4718/=0A= >>>>> Thanks to Konstantin's loop unrolling.=0A= >>>>>=0A= >>>>> Signed-off-by: Gerald Rogers =0A= >>>>> Signed-off-by: Huawei Xie =0A= >>>>> Acked-by: Konstantin Ananyev =0A= >>>>> ---=0A= >>>>> lib/librte_mbuf/rte_mbuf.h | 50 ++++++++++++++++++++++++++++++++++++= ++++++++++=0A= >>>>> 1 file changed, 50 insertions(+)=0A= >>>>>=0A= >>>>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h= =0A= >>>>> index f234ac9..4e209e0 100644=0A= >>>>> --- a/lib/librte_mbuf/rte_mbuf.h=0A= >>>>> +++ b/lib/librte_mbuf/rte_mbuf.h=0A= >>>>> @@ -1336,6 +1336,56 @@ static inline struct rte_mbuf *rte_pktmbuf_all= oc(struct rte_mempool *mp)=0A= >>>>> }=0A= >>>>>=0A= >>>>> /**=0A= >>>>> + * Allocate a bulk of mbufs, initialize refcnt and reset the fields = to default=0A= >>>>> + * values.=0A= >>>>> + *=0A= >>>>> + * @param pool=0A= >>>>> + * The mempool from which mbufs are allocated.=0A= >>>>> + * @param mbufs=0A= >>>>> + * Array of pointers to mbufs=0A= >>>>> + * @param count=0A= >>>>> + * Array size=0A= >>>>> + * @return=0A= >>>>> + * - 0: Success=0A= >>>>> + */=0A= >>>>> +static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,= =0A= >>>>> + struct rte_mbuf **mbufs, unsigned count)=0A= >>>>> +{=0A= >>>>> + unsigned idx =3D 0;=0A= >>>>> + int rc;=0A= >>>>> +=0A= >>>>> + rc =3D rte_mempool_get_bulk(pool, (void **)mbufs, count);=0A= >>>>> + if (unlikely(rc))=0A= >>>>> + return rc;=0A= >>>>> +=0A= >>>>> + switch (count % 4) {=0A= >>>>> + while (idx !=3D count) {=0A= >>>>> + case 0:=0A= >>>>> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) =3D=3D 0);=0A= >>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1);=0A= >>>>> + rte_pktmbuf_reset(mbufs[idx]);=0A= >>>>> + idx++;=0A= >>>>> + case 3:=0A= >>>>> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) =3D=3D 0);=0A= >>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1);=0A= >>>>> + rte_pktmbuf_reset(mbufs[idx]);=0A= >>>>> + idx++;=0A= >>>>> + case 2:=0A= >>>>> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) =3D=3D 0);=0A= >>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1);=0A= >>>>> + rte_pktmbuf_reset(mbufs[idx]);=0A= >>>>> + idx++;=0A= >>>>> + case 1:=0A= >>>>> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) =3D=3D 0);=0A= >>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1);=0A= >>>>> + rte_pktmbuf_reset(mbufs[idx]);=0A= >>>>> + idx++;=0A= >>>>> + }=0A= >>>>> + }=0A= >>>>> + return 0;=0A= >>>>> +}=0A= >>>> This is weird. Why not just use Duff's device in a more normal manner.= =0A= >>> But it is a sort of Duff's method.=0A= >>> Not sure what looks weird to you here?=0A= >>> while () {} instead of do {} while();?=0A= >>> Konstantin=0A= >>>=0A= >>>=0A= >>>=0A= >> It is unusual to have cases not associated with block of the switch.=0A= >> Unusual to me means, "not used commonly in most code".=0A= >>=0A= >> Since you are jumping into the loop, might make more sense as a do { } w= hile()=0A= > I find this a very odd coding practice and I would suggest we not do this= , unless it gives us some great performance gain.=0A= >=0A= > Keith=0A= The loop unwinding could give performance gain. The only problem is the=0A= switch/loop combination makes people feel weird at the first glance but=0A= soon they will grasp this style. Since this is inherited from old famous=0A= duff's device, i prefer to keep this style which saves lines of code.=0A= >>=0A= >=0A= > Regards,=0A= > Keith=0A= >=0A= >=0A= >=0A= >=0A= =0A=