Re: [dpdk-dev] [PATCH v2 1/2] mbuf: provide rte_pktmbuf_alloc_bulk API

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2015-12-21 17:20, Wiles, Keith:
> On 12/21/15, 9:21 AM, "Xie, Huawei" <huawei.xie@intel.com> wrote:
> >On 12/19/2015 3:27 AM, Wiles, Keith wrote:
> >> On 12/18/15, 11:32 AM, "dev on behalf of Stephen Hemminger" <dev-bounces@dpdk.org on behalf of stephen@networkplumber.org> wrote:
> >>> On Fri, 18 Dec 2015 10:44:02 +0000
> >>> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> >>>>> On Mon, 14 Dec 2015 09:14:41 +0800
> >>>>> Huawei Xie <huawei.xie@intel.com> 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