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

["Xie, Huawei" <huawei.xie@intel.com>]

On 12/22/2015 5:32 AM, Thomas Monjalon wrote:
> 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.
OK. Will add more context. Probably the wiki page for duff's device
should be updated on how to handle the case count is zero, using while()
or add one line to check.

> Thanks
>