From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v2 1/2] mbuf: provide rte_pktmbuf_alloc_bulk API Date: Mon, 21 Dec 2015 22:30:39 +0100 Message-ID: <53745737.WGjHJuKom7@xps13> References: <1450049754-33635-1-git-send-email-huawei.xie@intel.com> <868B1A38-37CB-4E19-B3FE-E9B74C35545B@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org To: "Xie, Huawei" Return-path: Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by dpdk.org (Postfix) with ESMTP id F2377F72 for ; Mon, 21 Dec 2015 22:32:12 +0100 (CET) Received: by mail-wm0-f50.google.com with SMTP id p187so84750068wmp.0 for ; Mon, 21 Dec 2015 13:32:12 -0800 (PST) In-Reply-To: <868B1A38-37CB-4E19-B3FE-E9B74C35545B@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2015-12-21 17:20, Wiles, Keith: > On 12/21/15, 9:21 AM, "Xie, Huawei" wrote: > >On 12/19/2015 3:27 AM, Wiles, Keith wrote: > >> On 12/18/15, 11:32 AM, "dev on behalf of Stephen Hemminger" wrote: > >>> On Fri, 18 Dec 2015 10:44:02 +0000 > >>> "Ananyev, Konstantin" wrote: > >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger > >>>>> On Mon, 14 Dec 2015 09:14:41 +0800 > >>>>> Huawei Xie wrote: > >>>>>> + switch (count % 4) { > >>>>>> + while (idx != count) { > >>>>>> + case 0: > >>>>>> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); > >>>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1); > >>>>>> + rte_pktmbuf_reset(mbufs[idx]); > >>>>>> + idx++; > >>>>>> + case 3: > >>>>>> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); > >>>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1); > >>>>>> + rte_pktmbuf_reset(mbufs[idx]); > >>>>>> + idx++; > >>>>>> + case 2: > >>>>>> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); > >>>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1); > >>>>>> + rte_pktmbuf_reset(mbufs[idx]); > >>>>>> + idx++; > >>>>>> + case 1: > >>>>>> + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); > >>>>>> + rte_mbuf_refcnt_set(mbufs[idx], 1); > >>>>>> + rte_pktmbuf_reset(mbufs[idx]); > >>>>>> + idx++; > >>>>>> + } > >>>>>> + } > >>>>>> + return 0; > >>>>>> +} > >>>>> This is weird. Why not just use Duff's device in a more normal manner. > >>>> But it is a sort of Duff's method. > >>>> Not sure what looks weird to you here? > >>>> while () {} instead of do {} while();? > >>>> Konstantin > >>>> > >>>> > >>>> > >>> It is unusual to have cases not associated with block of the switch. > >>> Unusual to me means, "not used commonly in most code". > >>> > >>> Since you are jumping into the loop, might make more sense as a do { } while() > >> I find this a very odd coding practice and I would suggest we not do this, unless it gives us some great performance gain. > >> > >> Keith > >The loop unwinding could give performance gain. The only problem is the > >switch/loop combination makes people feel weird at the first glance but > >soon they will grasp this style. Since this is inherited from old famous > >duff's device, i prefer to keep this style which saves lines of code. > > Please add a comment to the code to reflex where this style came from and why you are using it, would be very handy here. +1 At least the words "loop" and "unwinding" may be helpful to some readers. Thanks